Skip to content

1 - Conditional DELETE: Why the Condition Must Be Evaluated Once

This document records the analysis, design discussion, and repair decision for SILO PR #12.

Status on 2026-08-26: PR #12 remains open at head 5b71a75e, 118 commits behind the latest main. Its commit has no DCO sign-off and GitHub reports no check runs. The improved design described here has been implemented, tested, and reviewed twice in an isolated local worktree, but it has not been committed, pushed, merged, or released.
Scope: correctly support If-Match for the single-object DeleteObject API, and fail closed instead of silently deleting when an unsupported per-object DeleteObjects ETag is received. Full batch-condition execution and bucket-policy enforcement remain separate deliverables.
Release boundary: local implementation, tests, review, commit, push, remote CI, merge, tag, image publication, and production deployment are independent gates.

Too Long; Didn’t Read (TL;DR)

The underlying problem is real. SILO currently ignores If-Match on DELETE, so a client can believe it is performing compare-and-delete while the server performs an unconditional deletion. PR #12 targets the right problem and correctly recognizes that the condition must be evaluated against fresh object state while holding a lock.

The original implementation puts the same HTTP callback into every erasure pool. Different pools can retain copies from different points in time, so each pool evaluates and mutates against its own ETag. A two-pool test reproduced both failures:

  • the request ultimately returns 412 after the older matching copy has already been deleted;
  • the request succeeds after deleting the current copy, while an older non-matching copy remains and becomes visible again.

The selected repair introduces no new condition framework. It follows the established multi-pool GET pattern: select the current object under the outer namespace lock, evaluate the condition exactly once, then clear the callback before calling lower pools. A false condition mutates no pool. A true condition allows the existing cleanup to run without reinterpreting the client condition per copy.

Why this is a real problem

Silently dropping the condition is unsafe

AWS conditional-delete documentation now defines the behavior for general-purpose buckets and both DeleteObject and DeleteObjects:

Request Meaning Result and permission
If-Match: <ETag> Delete only if the current object is still the state observed by the caller 204 on match, 412 otherwise; requires s3:GetObject and s3:DeleteObject
If-Match: * Delete only if a current object exists 204 when it exists; requires only s3:DeleteObject
Missing key No condition can be satisfied Not Found
Current delete marker No current object exists If-Match: * returns 412

Ignoring the header is therefore not a harmless unsupported extension. It removes the concurrency guard the caller used to avoid deleting another writer’s newer object.

Not every S3 client sends conditional deletes, so prevalence is unknown. Severity for each relying caller is high: one silent downgrade can remove newly committed data.

A delete marker is not merely an ETag comparison case

PR #12 reuses the generic isETagEqual, which returns true whenever the right-hand value is *. Consequently, isETagEqual("", "*") is also true.

The more fundamental bypass occurs one layer above. erasureServerPools.DeleteObject returns success immediately when the current object is already a delete marker. That happens before the callback added by the PR. A diagnostic test observed zero callback calls and a successful result.

The impact needs precise wording:

  • the delete-marker fast path did not remove a historical version or create another marker in the reproduced case; it bypassed the condition and falsely reported success;
  • the multi-pool counterexamples do mutate storage on a failed request or leave a stale copy after success.

Changing only isETagEqual("", "*") cannot cross the outer fast path and risks altering a comparator shared by GET, PUT, and COPY.

What the original PR got right

Its high-level algorithm is sound:

  1. detect If-Match in the handler;
  2. read fresh ObjectInfo after acquiring the storage lock;
  3. return before mutation when the condition is false;
  4. encode the result as an S3 response.

This avoids the obvious TOCTOU window of a separate HEAD followed by DELETE. The PR also adds handler, helper, and erasure-layer tests. Its ordinary single-pool path correctly returns 412 and preserves the object for a wrong specific ETag.

The defect is not the decision to evaluate under a lock. It is choosing the wrong layer and therefore the wrong object state.

Where the atomicity boundary lives

The deletion path has two layers:

DeleteObjectHandler
    -> erasureServerPools.DeleteObject
         holds the namespace write lock
         selects the latest current pinfo across pools
         handles the delete-marker fast path
         selects a single-pool or all-pool deletion
             -> erasureSets / erasureObjects.DeleteObject

Only erasureServerPools.DeleteObject knows:

  • which copy represents the current object;
  • which pools still contain older copies or inconsistent metadata;
  • whether the delete-marker fast path applies;
  • whether multiple pools will be mutated concurrently.

The client condition therefore belongs at this layer. A single pool knows only its local copy and cannot reinterpret a condition on the logical current object.

Two-pool counterexamples

The test places an older object in pool 0 and a newer object with a different ETag in pool 1. Reads select pool 1 as current, while an unversioned delete cleans both pools.

Condition matches the old copy

The original PR lets pool 0 pass and delete its copy while pool 1 fails. The aggregate result follows the current pool and returns 412, even though storage changed.

Condition matches the current copy

Pool 1 passes and deletes the current copy. Pool 0 fails and retains the old copy. The request returns success, after which the old object becomes visible again.

The callback also captures a single http.ResponseWriter. Calling it concurrently from multiple pools can make multiple goroutines write the same HTTP response. Storage replicas should not concurrently decide wire-level output.

A pre-existing degraded-pool limitation

There is one related but inherited limitation outside this patch. If the selected current pool is readable and writable but an older, non-current pool is degraded, the existing all-pool delete path can return the selected pool’s success while an error from the older pool is not surfaced. That copy can remain and reappear after recovery.

The new condition does not create this behavior: it evaluates the readable current object correctly and then enters the same unversioned multi-pool cleanup used by an unconditional delete. Repairing error aggregation and recovery for partially degraded old pools should be tracked separately because it changes the guarantees of every unversioned multi-pool delete, not only conditional requests.

The selected minimal repair

1. Evaluate exactly once at the outer layer

After erasureServerPools.DeleteObject acquires the namespace write lock:

  1. save opts.CheckPrecondFn;
  2. remove it from options passed to lower layers;
  3. inspect all pools and select the current pinfo;
  4. if the current object cannot be read reliably, return a quorum error without calling the callback;
  5. call the saved callback exactly once with pinfo.ObjInfo;
  6. on success, continue through the existing deletion path with no lower-layer reinterpretation.

This pattern already exists in multi-pool GetObjectNInfo: save the callback, clear it below, select the latest object, and evaluate once. Reusing it limits the DELETE change to the real atomicity boundary.

2. Treat * as current-representation existence

The DELETE-specific check separates wildcard and ETag semantics:

specific ETag -> compare the client-visible ETag of the current object
*             -> require a non-empty object name and no delete marker

A missing key already returns Not Found during object selection. A current delete marker reaches the callback and returns 412. The generic isETagEqual remains unchanged.

3. Require read permission for a specific ETag

The handler first checks s3:DeleteObject. When the normalized condition is not a bare *, it additionally checks s3:GetObject:

  • delete-only policy plus *: allowed;
  • delete-only policy plus a specific ETag: 403 and no mutation;
  • Get plus Delete and a matching ETag: allowed.

Authorization completes before any storage mutation.

4. Do not require SSE-C content decryption for DELETE

The original PR invokes the GET/PUT-oriented DecryptObjectInfo, which rejects an SSE-C object when SSE-C read headers are absent. Conditional DELETE needs the client-visible ETag, not plaintext content or decrypted size.

The selected implementation uses the established getDecryptedETag projection only for a specific ETag. Wildcard requests do not read the ETag. This reuses existing ETag behavior without imposing content-decryption requirements on DELETE.

5. Evaluate the current version

AWS specifies that conditional-delete evaluation applies to the current version. SILO’s outer pool selection already reads the current object, while preserving an explicit versionId for the eventual version deletion.

A regression test requests deletion of a historical version while matching that historical ETag rather than the current ETag. It must return 412 and preserve both versions.

6. Reject silent downgrades at unsupported edges

Two small guards keep the single-object feature from being bypassed:

  • an empty or whitespace-only If-Match is rejected instead of becoming an unconditional delete;
  • If-Match cannot be combined with the internal recursive x-minio-force-delete extension, whose prefix semantics cannot represent one object’s ETag condition; the HTTP handler rejects it and the storage layer also refuses any internal prefix-delete plus callback combination.

The batch XML decoder now also recognizes per-object <ETag> values. Until atomic per-item execution is implemented, any non-empty batch ETag rejects the entire request with NotImplemented before deletion begins. This is not batch conditional-delete support; it is a narrow data-safety guard against silently discarding a condition.

Rejected alternatives

Change only isETagEqual

It does not address the outer delete-marker fast path and risks changing several APIs that share the comparator.

Keep per-pool callbacks and aggregate the result

An aggregate error cannot roll back a copy already deleted by another pool. The condition applies to the logical current object, not independently to every physical copy.

Introduce a new condition object or transaction coordinator

The current feature has one If-Match condition, and CheckPrecondFn already expresses it. GET demonstrates the correct one-shot consumption pattern. A new DSL, state machine, or cross-pool transaction abstraction is unnecessary.

Complete every conditional-delete feature in one PR

DeleteObjects and policy conditions cross different API and repository boundaries. Combining XML parsing, per-item responses, IAM, quiet mode, and dependency publication with the core deletion repair would make the change harder to validate.

Test and acceptance contract

The minimally sufficient matrix is:

Layer Evidence
Condition helper matching, mismatching, quoted ETag, wildcard, delete marker, non-DELETE method, and SSE-C client-visible ETag projection without content-decryption headers
Handler wrong ETag returns 412 and preserves the object; matching ETag returns 204; missing key returns Not Found; blank conditions and conditional force-delete are rejected without mutation
Permission delete-only plus specific ETag returns 403 and preserves the object; the same policy plus * succeeds
Single-pool storage matching/mismatching condition, missing object, delete marker, one callback call, and refusal of a conditional prefix delete
Quorum unreadable current object returns a quorum error, calls the callback zero times, and remains after disks recover
Versioning a historical versionId condition still evaluates the current version
Two pools 412 changes no pool; 204 removes all copies; one callback call in both cases
Batch safety guard an unsupported per-object <ETag> returns NotImplemented and preserves every object

The original PR’s quorum test merely took 8 of 16 disks offline and asserted that some error occurred. Delete write quorum was already unavailable, so the same test passed on main without conditional DELETE. The replacement asserts the specific quorum result, zero callback calls, and object survival after restoring the disks.

Independent adversarial review

Two read-only local Claude Code reviews used the Fable model at xhigh effort against the exact server diff and both design records. Both verdicts were GO WITH NON-BLOCKING NOTES, with no P0, P1, or P2 findings after the first round’s changes were applied.

The first review found the conditional force-delete bypass, whitespace-only downgrade, silent batch-ETag discard, missing versioned-success coverage, and the inherited degraded-old-pool limitation. Those findings produced the guards, tests, and limitation text above. The second review confirmed the outer atomicity boundary, error handling, auth split, batch-field blast radius, response-writer behavior, bilingual parity, and minimality. Its remaining actionable P3 was a hypothetical internal caller combining prefix deletion with a callback; the storage layer now rejects that combination too.

One reviewer sentence suggested that SSE-C without customer-key headers would necessarily fail the condition. Direct inspection showed the opposite established behavior: getDecryptedETag projects the stored client-visible suffix without asking to decrypt object contents. A focused regression test now pins that behavior. Remaining non-blocking notes are multiple-header normalization and the deliberate 501-before-auth error-ordering nuance. A live-AWS differential check for specific ETag versus a current delete marker and versionId plus If-Match would still be useful before claiming byte-for-byte behavioral parity beyond the published contract.

Deliberate follow-up scope

Per-object conditions in DeleteObjects

The AWS DeleteObjects API accepts an <ETag> per <Object> and returns each outcome under <Deleted> or <Error> in the same 200 response.

The safety patch adds an ETag field to ObjectToDelete only so the handler can detect the condition and reject the entire request before mutation. This closes the previous silent unconditional-delete behavior, but it does not implement AWS’s required per-object evaluation or mixed <Deleted> / <Error> response.

Full compatibility remains a separate high-priority change: evaluate every item against the logical current object under the correct lock, apply the exact-ETag permission rule per item, preserve quiet-mode behavior, and report each failed condition without blocking unrelated items.

The s3:if-match policy condition key

AWS policies can enforce conditional deletes. SILO’s silo-pkg does not yet define s3:if-match. Full support requires:

  1. the condition key and action map in silo-pkg;
  2. a new silo-pkg release;
  3. correct condition values for a single-delete header and batch per-item ETags;
  4. a server dependency update and policy compatibility tests.

That is a separate cross-repository deliverable, not a prerequisite for making single-object execution correct.

Complexity, benefit, and cost

Production code remains small: one DELETE-specific condition helper, one extra authorization check, roughly a dozen lines that consume the callback once at the outer layer, and narrow fail-closed guards for malformed/recursive and as-yet unsupported batch conditions. Most complexity belongs in tests because deletion spans pools, versions, markers, quorum, and permissions.

Scope Complexity Main cost
This single-object repair plus batch safety guard Medium Regression coverage across the destructive hot path
Batch conditional delete Medium-high XML, per-item conditions, mixed responses, quiet mode
Policy condition key Medium and cross-repository silo-pkg release, server condition values, policy tests

The benefit exceeds the cost. It removes a dangerous silent unconditional delete and places the condition at an existing global consistency boundary. Reusing the current outer-lock/latest-object pattern is the minimal, sufficient, and necessary design.

Merge and release gates

The single-object repair becomes mergeable only after:

  1. targeted condition, permission, versioning, quorum, and two-pool tests pass;
  2. go test ./cmd, go vet ./cmd, formatting, and diff checks pass;
  3. an independent adversarial review has no unresolved blocker;
  4. the contribution is organized on current main with a valid author DCO sign-off;
  5. DCO, Go CI, VulnCheck, and other required remote workflows are green;
  6. the PR description distinguishes complete DeleteObject support from the batch fail-closed guard and links the full batch/policy follow-ups.

A merge is still not a release. Users can rely on the behavior only after a corresponding SILO release, package, docker.io/pgsty/minio image, deployment, and real-client verification have independently completed.

Conclusion

Conditional DELETE is worth implementing. PR #12 has the right goal and the useful insight that fresh state must be checked under a lock. The required correction is the boundary: a client condition belongs to the logical current object and cannot be interpreted independently by every physical copy.

The selected design moves one callback to the erasureServerPools layer that already selects the current object, preserves the generic comparator, handles wildcard/delete-marker semantics explicitly, and adds the specific-ETag read permission. It changes no storage format, dependency, or public condition framework. The batch change is deliberately limited to refusing an unsupported condition before mutation; full batch execution and policy support remain separate work.

That is the minimum complexity needed to make the feature sufficient and safe.

2 - DSN-Only Database Notifications: A Compatibility Boundary for #53

This document is the product requirements and final design record for SILO issue #53. It records the accepted compatibility boundary, implementation, and verification for PostgreSQL and MySQL bucket-notification targets.

Decision

SILO will retain PostgreSQL and MySQL notification targets, but support exactly one current configuration form for each:

  • PostgreSQL requires a complete connection_string.
  • MySQL requires a complete dsn_string.

The old five-field form — host, port, username, password, and database — remains unsupported by the current KV configuration system. SILO will not re-register those keys and will not synthesize a DSN from them during legacy migration.

The legacy migration contract is deliberately narrow:

Legacy target Result
Disabled Ignore it; no target is emitted.
Enabled with a non-empty connection_string or dsn_string Migrate only the canonical connection-string key and the other registered target settings.
Enabled with only discrete connection fields Reject migration and abort server startup before the new configuration is activated, with an actionable error that names the subsystem and target but never prints a credential.

This is a configuration-boundary decision, not removal of the database-notification feature.

Status: implemented in server commit f1ba68358; release pending.
Owner: SILO server repository.
Tracking: pgsty/silo#53.
Target: the next SILO patch release after implementation and verification.

Context

SILO inherited two generations of database-notification configuration from MinIO.

The pre-KV JSON configuration could describe a database connection either as a complete string or as five fields:

host
port
username
password
database

The current KV configuration exposes only the driver-native form:

notify_postgres  -> connection_string
notify_mysql     -> dsn_string

This direction is not new. MinIO deprecated the five discrete fields in RELEASE.2020-04-10T03-34-42Z and instructed operators to move to connection_string or dsn_string. SILO’s current help tables, environment-variable documentation, and examples already present the complete string as the supported interface.

SILO is a new community fork with an explicit migration step. Its compatibility contract prioritizes the S3 and Admin APIs, current MINIO_* settings, on-disk data, and current KV configuration. It does not need to perpetuate every pre-2020 configuration spelling when a supported canonical form has existed for years.

The defect

Before the fix, the legacy migration helpers, SetNotifyPostgres and SetNotifyMySQL, wrote both forms into the new KV configuration. Even when the old target already had a complete connection string, the helpers also emitted all five discrete keys, usually with empty values.

The new parser rejects those keys because neither DefaultPostgresKVS nor DefaultMySQLKVS registers them. Key validation checks key presence, not whether the corresponding value is empty. Both legacy source forms therefore fail:

old complete string -> canonical string + five empty unknown keys -> rejected
old discrete fields -> empty canonical string + five populated unknown keys -> rejected

The failure is amplified by notification initialization. FetchEnabledTargets is fail-fast across notification subsystems: the first invalid subsystem returns an error and a nil target list. The caller logs the error and continues starting the object server, leaving healthy Webhook, Kafka, NATS, and other targets unavailable as well.

Merely returning an error from the two migration helpers does not fix that behavior. The error propagates through readConfigWithoutMigrate and initConfig, but initConfigSubsystem currently logs non-retriable configuration errors as “some features may be missing” and returns success. The server then starts without assigning globalServerConfig; notification failure is only one consequence, because region, storage class, compression, identity, and other stored settings may also be absent. The implementation must therefore carry a typed database-migration error to the startup boundary and make that error fatal. Classifying it as retriable is also wrong because the server would retry forever without any state change that could repair the configuration.

The resulting behavior is especially dangerous because object I/O still works. Operators can see a healthy S3 service while every configured event pipeline has stopped. Targets are never constructed, so delivery or later replay of events produced during the outage must not be assumed.

There is also a diagnostic-exposure issue. The unregistered password key has no sensitivity metadata and may be copied verbatim into health or diagnostic material. The registered connection_string and dsn_string keys are already treated as sensitive values.

Why the first fix was reverted

The first repair registered the five discrete keys and taught the parser to read them. That made migrated targets pass CheckValidKeys, and it appeared attractive because the target argument structures and constructors still contain code for the old fields.

It also broke the documented connection-string path.

The shared mc admin config set tokenizer discovers field boundaries by looking for registered key names. It is not fully quote-aware. Once port became a registered key, this valid input contained what looked like a second top-level field:

connection_string="host=db port=5432 dbname=events user=app"

The tokenizer split at the port= inside the quoted value, truncated connection_string, and handed the remainder to the port parser. The command then failed with invalid port.

Under the current tokenizer, registering common words such as host, port, and password creates a direct conflict between the connection-string grammar and the top-level KV grammar. The attempted registration fix was therefore reverted. Re-registering those keys is not an acceptable solution.

Product judgment

Database notification targets are a specialized but useful capability. They provide a direct database-backed namespace view or access journal without requiring an external event bus. That remains valuable for small deployments and for users already operating PostgreSQL or MySQL.

The legacy spelling of their connection parameters has much less value. A five-field model cannot represent the useful range of driver options: TLS modes and certificates, connection timeouts, application names, Unix sockets, multi-host PostgreSQL settings, MySQL driver parameters, and future driver capabilities. Supporting both forms also creates precedence, merging, redaction, and testing questions that do not exist with one canonical value.

The complete string is the better abstraction boundary: SILO owns notification semantics, while the database driver owns connection syntax.

The product decision is therefore to keep the capability and remove the compatibility illusion. An unsupported legacy target must be rejected clearly; it must not be accepted and transformed into a configuration that later disables unrelated targets.

Goals

  1. Establish connection_string and dsn_string as the only supported live configuration interfaces for database notifications.
  2. Allow a legacy JSON target that already contains the canonical string to cross the migration boundary without modification to its connection semantics.
  3. Reject enabled discrete-only legacy targets before a partial or invalid KV configuration is activated.
  4. Replace the current silent runtime failure mode of #53 — healthy targets disabled while the server appears healthy — with an explicit startup-time failure that operators must resolve before the server runs.
  5. Ensure no migration error, log line, health report, or diagnostic bundle exposes a database password.
  6. Remove the ten Postgres/MySQL exceptions from the source-level unregistered-write audit.
  7. Make the compatibility boundary and operator remediation explicit in release and migration documentation.

Non-goals

  • Supporting both DSN and discrete database fields in the current KV interface.
  • Automatically synthesizing a DSN from old discrete fields.
  • Rewriting the shared KV tokenizer.
  • Changing FetchEnabledTargets fail-fast semantics in this patch.
  • Silently skipping an enabled database target and continuing with partial notification coverage.
  • Removing PostgreSQL or MySQL notification targets.
  • Deleting the legacy struct fields needed to decode and identify unsupported input. They remain on shared target argument structs that are also used by live constructors, whose discrete-field connection-string synthesis is unreachable from current KV configuration; those fields must not become supported configuration keys.
  • Correcting ignored errors from the other eight legacy notification setters. Their pre-existing silent-skip behavior remains unchanged in this narrowly scoped database-migration patch and requires a separate audit and design decision.

Functional requirements

Current configuration

  1. notify_postgres accepts connection_string; notify_mysql accepts dsn_string.
  2. The five discrete keys remain unregistered and rejected by current configuration commands.
  3. Existing full strings must continue to support the database driver’s syntax, including parameters whose names contain host, port, user, password, or database.
  4. No new public environment variables or KV keys are introduced.
  5. The declared legacy variables MINIO_NOTIFY_POSTGRES_HOST/PORT/USERNAME/PASSWORD/DATABASE and their MySQL equivalents are not wired into current parsing and remain unsupported. They must not be documented as working alternatives to the complete-string variables.

Legacy migration

  1. SetNotifyPostgres must return without emitting a target when the legacy target is disabled.
  2. For an enabled target, SetNotifyPostgres must require a non-empty ConnectionString and write only registered Postgres keys. If both a canonical string and discrete fields are present, the canonical string wins and every discrete value is discarded.
  3. SetNotifyMySQL must apply the equivalent rule to DSN.
  4. Neither helper may emit host, port, username, password, or database.
  5. A missing canonical string must return a typed or wrapped migration error identifying the subsystem and target name.
  6. cmd/config-migrate.go must check and propagate both helper errors. Ignoring them is forbidden.
  7. No partially migrated configuration may be activated or persisted after either helper fails.
  8. Error text may name the required key and remediation, but must not include any connection-field value.
  9. The propagated typed migration error must abort server startup. It must not be downgraded to the non-fatal “some features may be missing” path in initConfigSubsystem, and it must not enter the retriable-error loop.
  10. Validation errors for a supplied canonical string follow the same startup-fatal and secrecy rules; wrapping must add target context without repeating the DSN or its components.

Recommended error shape:

notify_postgres:archive uses unsupported legacy discrete connection fields;
set connection_string before migrating to SILO

Operator remediation

An operator encountering the error must choose an explicit remediation path. This applies both before an initial switch to SILO and when upgrading a deployment that is already running SILO: legacy migration output is not persisted, so the same old JSON source can re-enter migration on every start. A deployment that currently starts with notifications silently broken can therefore fail to start after this repair until the source configuration is corrected.

  1. On a compatible intermediate MinIO release, replace the old fields with connection_string or dsn_string, verify the target, and then migrate to SILO.
  2. Disable or remove the legacy database target, migrate the server, and recreate the target with the canonical string afterward.
  3. For a fresh SILO installation, create the target directly with the canonical string; no legacy migration is involved.
  4. For an existing SILO deployment that still reads a legacy JSON file, stop on the previous working release, back up the source configuration, then convert, disable, or remove the database target before starting the fixed release. Do not delete or rewrite unrelated configuration.

Documentation must not suggest that a discrete-only target will be converted automatically.

Availability trade-off

This decision intentionally turns one unsupported configuration from a degraded startup into a hard startup failure. The immediate availability cost is real: a server that previously served objects while all notifications were silently dead may refuse to start after the repair.

That cost is accepted because an object server that appears healthy while configured event sinks are absent creates silent, potentially unrecoverable downstream data loss. SILO is a new fork with an explicit migration boundary, and the discrete form has been deprecated since 2020. A fatal, actionable precondition is preferable to an upgrade that reports success with reduced notification coverage. The release note must make this startup behavior prominent; it must not be buried as an internal migration cleanup.

Security requirements

  1. The unsupported-input error must never format the legacy argument structure or its values.
  2. Tests must use a sentinel password and assert that it is absent from returned errors and captured logs.
  3. Migrated output must contain the registered sensitive connection-string key and no standalone password key.
  4. If a diagnostic bundle was exported from an affected deployment before this repair, operators should treat the database password as potentially disclosed and rotate it.

Alternatives considered

Register and parse the discrete fields

Benefit: preserves the old source form and uses already existing argument fields.
Rejected because: registration makes common field names visible to the shared tokenizer and corrupts quoted connection strings. It also expands the supported public configuration surface after the fields were deprecated in 2020.

Synthesize a canonical string during migration

Benefit: preserves discrete-only legacy installations.
Rejected because: it creates permanent code and test ownership for an obsolete input form, including PostgreSQL quoting, MySQL DSN formatting, socket and IPv6 behavior, defaults, and future driver drift. For a new fork with an explicit migration boundary, the benefit does not justify the continuing surface.

Skip only the unsupported target

Benefit: keeps the object server and other notification targets running.
Rejected because: silently discarding a configured event sink can cause unobservable and unrecoverable event loss. A clear migration failure is safer than an apparently successful upgrade with reduced notification coverage.

Change global notification fail-fast behavior

Benefit: limits the blast radius of future invalid targets.
Rejected for this change because: it neither repairs the database target nor closes the credential-exposure path, and it changes system-wide error semantics. It may be evaluated independently with its own operational contract.

Remove database notification targets

Benefit: removes the complete database-specific maintenance surface.
Rejected because: the targets remain useful and self-contained. The defect belongs to an obsolete configuration form, not to the notification capability itself.

Implementation scope

The server change should remain narrow:

  1. Update internal/config/notify/legacy.go so the two database setters emit only canonical registered keys and reject enabled targets without a canonical string.
  2. Update cmd/config-migrate.go to propagate the two database-helper errors with subsystem and target context.
  3. Define a typed database-migration error and update cmd/server-main.go so initConfigSubsystem returns it as fatal instead of logging and ignoring it. It must remain non-retriable.
  4. Leave ignored errors from the other eight legacy notification setters unchanged in this patch; record them for a separate audit rather than expanding #53 implicitly.
  5. Remove all ten Postgres/MySQL entries from knownUnregisteredWrites; the ratchet should become empty unless another independently justified legacy exception exists.
  6. Add focused migration, startup, validation, secrecy, and coexistence tests.
  7. Update database-notification and migration documentation in silo.pgsty.com.

The patch must not register the old keys, change the generic tokenizer, or refactor unrelated notification targets.

Acceptance criteria

The implementation is complete only when all of the following are demonstrated:

  1. A legacy PostgreSQL target with a complete connection string migrates, passes CheckValidKeys, and is returned by GetNotifyPostgres unchanged.

  2. A legacy MySQL target with a complete DSN does the equivalent.

  3. Discrete-only enabled targets for both databases fail before target initialization with an actionable error containing the subsystem and target name, and server startup aborts.

  4. Missing-string and malformed-string errors contain none of the sentinel host, username, password, database, or DSN values.

  5. Disabled discrete legacy targets do not create configuration entries and do not block migration.

  6. Migrated KVS output contains none of the ten discrete keys, including empty ones.

  7. When a legacy target contains both a canonical string and conflicting discrete values, only the canonical string is migrated and no discrete sentinel appears in any output KVS value.

  8. A SetKVS regression test using the real DefaultPostgresKVS and DefaultMySQLKVS key sets accepts a quoted connection string containing port=, host=, or password=.

  9. A configuration containing healthy Webhook, Kafka, or NATS targets cannot reach FetchEnabledTargets with an invalid migrated database target because readConfigWithoutMigrate fails without yielding, persisting, or activating a partial configuration, and startup aborts on that typed error.

  10. initConfigSubsystem returns the typed migration error; it neither logs-and-continues nor enters the retriable loop.

  11. knownUnregisteredWrites no longer contains Postgres or MySQL exceptions.

  12. The following verification passes:

    go test ./internal/config/notify ./internal/config ./internal/event/target -count=1
    go test -v ./cmd -run 'Test(ReadConfigWithoutMigrate|InitConfigSubsystem)' -count=1
    git diff --check

    The verbose cmd output must show that tests with both prefixes actually ran; a zero-match warning is a failed acceptance check. The normal server CI suite must also pass. In the documentation checkout, run make check.

Implementation result

Server commit f1ba68358 implements the accepted design without expanding the public configuration surface:

  • the two legacy database setters emit only connection_string or dsn_string plus registered target settings;
  • disabled targets remain ignored, while enabled targets without a canonical string return a value-free LegacyDatabaseTargetError;
  • only the two database migration errors are newly propagated;
  • the typed error is non-retriable, escapes initConfigSubsystem, and is classified as fatal by serverMain before logger.FatalIf exits the process;
  • the ten Postgres/MySQL exceptions were removed from knownUnregisteredWrites;
  • focused tests cover complete-string round trips, canonical precedence, discarded discrete values, secrecy, failed-migration atomicity, startup classification, and the real tokenizer key sets.

The final local Claude Code review used Claude Fable 5 at max effort and returned GO with high confidence and no blocking findings. Verification included the focused package set, race tests, go vet ./cmd, and the complete go test ./cmd -count=1 suite. The review authorized only the six-file server commit; publication remains a separate gate.

Cross-repository review found no implementation changes are required in pgsty/mc, pgsty/silo-pkg, or pgsty/silo-console: the client forwards configuration text, the package repository owns no notification schema, and Console already serializes its form into the canonical connection_string or dsn_string. The public reference and compatibility documentation is updated with this record.

Release and compatibility statement

The release note must describe this as an enforced compatibility boundary:

SILO database notification targets require connection_string for PostgreSQL and dsn_string for MySQL. The pre-2020 discrete host/port/username/password/database form is not migrated. Convert or recreate such targets before switching the deployment to SILO.

Deployments already running SILO with an old-format source configuration are equally affected: after this release the server will not start until each enabled legacy database target is converted, disabled, or removed.

The issue should close only after the repair is present in a published server tag. A merged patch, a local site build, and a published release are separate completion gates.

Review record

Claude Fable 5 reviewed the first draft at xhigh effort on 2026-08-23 and returned approve with required changes. The required calibration was incorporated: startup-fatal propagation now extends through initConfigSubsystem; already-running SILO deployments are covered; the availability trade-off is explicit; canonical-string precedence, dead legacy environment variables, other ignored helper errors, and executable tests are specified.

The same model then completed a final source-backed verification pass. Final verdict: approve, with no blocking findings. It confirmed that the English and Chinese records are aligned, the requirements are implementable against the current server tree, and the acceptance criteria cover the startup, migration, parser-regression, and secrecy boundaries.

After implementation, a separate local Claude Code review using Claude Fable 5 at max effort traced the path through ExitFunc(1), inspected driver error behavior, ran the focused, race, vet, and full cmd suites, and returned GO with high confidence and no blocking findings.

3 - Preview Text, Never Execute It: SILO Console Text Preview PRD

Status: accepted design; implementation pending · Owner: pgsty/silo-console · Tracking: pgsty/silo#17 · Review: consensus of product, security, and frontend architecture reviews

SILO Console can preview images, PDFs, audio, and video, but not the small logs, text files, JSON documents, and XML documents that operators inspect every day. A correctly stored Content-Type does not help: these objects are classified as unsupported before the preview renderer is selected.

Restoring the old browser-native behavior would be easy. It would also be the wrong fix. An object in storage is controlled by the user who uploaded it. Loading that object as a same-origin HTML or XML document would turn a convenience feature into an execution boundary.

The accepted design therefore makes a stronger promise:

SILO previews eligible objects as bounded UTF-8 text. It never asks the browser to interpret their markup, MIME type, or file contents as a document.

This record fixes the product boundary, the resource limit, the security invariants, the implementation shape, and the evidence required before the feature can ship.

Decision

The first release will add a dedicated text preview type and a PreviewText component.

The contract is:

  1. Preserve every existing image, PDF, audio, and video classification.
  2. Only when the existing classifier returns none, consider a text fallback.
  3. Admit the four target extensions or four exact passive text MIME types.
  4. Fetch bytes through the ordinary authenticated download path, without preview=true.
  5. Enforce a hard application read limit of 1 MiB.
  6. Decode only strict UTF-8 and reject binary-looking content.
  7. Render one React text node inside a scrollable <pre>.
  8. Never use an iframe, HTML parser, XML parser, or HTML injection API.
  9. Show the complete object or no object; do not show a truncated JSON or XML document.
  10. Keep download available for files that are too large, invalidly encoded, or otherwise unavailable.

No Console API or S3 API change is required. The backend inline MIME allowlist is not expanded.

Current behavior

The defect is present in SILO Console v2.1.1, the version currently pinned by SILO when this design was written.

The frontend preview union contains only:

image | pdf | audio | video | none

Its extension table contains media formats, but not .log, .txt, .json, or .xml. Its MIME classifier likewise ignores text/plain, application/json, application/xml, and text/xml.

Runtime verification produced this split:

Object Frontend result Console download response
.log / text/plain none inline, SAMEORIGIN
.txt / text/plain none inline, SAMEORIGIN
.json / application/json “Preview unavailable” inline, SAMEORIGIN
.xml / application/xml none attachment, DENY

The object-detail action also uses the wrong conjunction when deciding whether Preview should be disabled. An authorized user can click Preview for an unsupported object and receive only the unavailable message; in other combinations, the UI can offer an action before the server rejects it.

The preview component still contains a generic same-origin iframe fallback. It is unreachable under the current type union, so the current defect is not an exploitable text-preview XSS. The dead branch is nevertheless hazardous: adding text to the union and letting it fall through would reactivate precisely the document-loading behavior this design rejects.

Root cause

This is contract drift across three independently evolved layers.

Classification drift

The browser code decides eligibility from filename and object metadata, but its closed type union has no text representation. Correct metadata cannot select a renderer that does not exist.

Response-policy drift

The Console server separately decides whether a response may be inline. It still treats plain text and JSON as safe passive MIME types, while XML and HTML remain attachments. That server decision is not reflected in the frontend classifier.

Renderer drift

The old generic iframe remains after the set of reachable preview types became media-only. The code therefore suggests a capability that the type system can no longer invoke.

The repair must realign the three layers without making MIME metadata a security boundary.

Why same-origin iframe preview is rejected

X-Frame-Options: SAMEORIGIN is not a sandbox. It controls who may embed a response; it does not limit what code inside a same-origin frame can do.

If uploader-controlled HTML, XHTML, SVG, or active XML were ever served as an inline same-origin document, it could act with the Console origin. An HttpOnly cookie would prevent direct cookie reads, but it would not prevent authenticated same-origin requests. A permissive or accidentally widened MIME rule would then turn stored content into stored application code.

nosniff, Content Security Policy, and Content-Disposition remain useful defense in depth, but none replaces the core invariant:

untrusted object bytes
        |
        v
strict text decoder
        |
        v
React textContent

never:
iframe / innerHTML / DOMParser / XML parser / executable document

Product contract

The feature is a read-only text viewer, not a web previewer and not an online editor.

The user should be able to:

  • open a small eligible object from either the list or object-detail surface;
  • read whitespace-preserving source text in the existing preview modal;
  • select and copy text using browser-native behavior;
  • understand whether a failure is caused by size, encoding, permission, object replacement, or network error;
  • download the original bytes at any time.

The user must never be led to believe that:

  • formatted JSON is the stored object;
  • a partial XML document is complete;
  • replacement characters are original bytes;
  • an unsupported encoding has been decoded faithfully;
  • an active HTML/XML document has been safely “sanitized” and executed.

Goals and non-goals

Goals

  1. Preview small logs, text, JSON, and XML without a local download.
  2. Keep object content inert regardless of extension, MIME, or payload.
  3. Bound retained response bytes and rendered text to 1 MiB.
  4. Preserve the stored text rather than silently reformatting it.
  5. Keep list and detail actions consistent with permissions and type eligibility.
  6. Support current object versions and explicitly selected historical versions.
  7. Preserve anonymous-access and subpath-hosting behavior.
  8. Ship the feature in Console first, then consume that exact Console revision in SILO.

Non-goals

  • HTML or XHTML rendering.
  • XML parsing, XSLT, external entities, or schema validation.
  • Markdown rendering.
  • JSON pretty-printing.
  • YAML or CSV-specific behavior.
  • Editing or saving.
  • Syntax highlighting, line numbers, search, folding, ANSI rendering, or linkification.
  • Head, tail, or truncated previews for large objects.
  • Lossy decoding or automatic detection of GBK, UTF-16, Latin-1, or other encodings.
  • A new backend text-preview endpoint.
  • Changes to the existing SVG, media, PDF, download, share, or storage contracts.

An object such as notes.md may still be shown as raw text when its exact MIME type is text/plain. It does not gain Markdown semantics.

Eligibility contract

Eligibility is deliberately two-stage.

Stage 1: preserve the legacy media decision

Run the current image, PDF, audio, and video classifier unchanged. If it returns anything other than none, return that result.

This preserves historical behavior for conflicting filename and MIME combinations.

Stage 2: apply text fallback

Only after the legacy result is none:

  1. Reject final extensions .html, .htm, and .xhtml.

  2. Match the final filename extension case-insensitively against:

    • .log
    • .txt
    • .json
    • .xml
  3. Normalize Content-Type by removing parameters, trimming whitespace, and lowercasing it.

  4. Match the normalized MIME exactly against:

    • text/plain
    • application/json
    • application/xml
    • text/xml

An allowed extension or an allowed exact MIME is sufficient. Broad matches such as text/, substring tests, and application/+json are forbidden in this release.

The resulting matrix is normative:

Filename and MIME Result Reason
report.txt + image/png image Existing media decision wins.
report.json + application/pdf PDF Existing media decision wins.
server.LOG + application/octet-stream text Allowed extension, case-insensitive.
no extension + application/json; charset=utf-8 text Exact normalized MIME.
page.html + text/plain none Explicit active-extension exclusion.
page.txt + text/html text Extension admits it; HTML source remains inert text.
notes.md + text/plain text Exact MIME admits raw text, not Markdown rendering.
image.svg + image/svg+xml existing image path No new text or iframe path.

Filename and MIME affect product eligibility only. They never select an executable rendering mode.

Resource contract

The binary limit is:

MAX_TEXT_PREVIEW_BYTES = 1,048,576

Exactly 1 MiB is eligible. 1 MiB plus one byte is not.

Known sizes

  • If the selected version has a known size greater than the limit, do not request its body.
  • If its known size is zero, show the empty-file state.
  • If its known size is within the limit, begin a bounded request.
  • An absent size is not the same as zero; it enters the bounded unknown-size path.

The current list-to-modal handoff must therefore preserve undefined rather than converting it to zero with a truthy fallback.

Bounded request

For a small or unknown size, request:

Range: bytes=0-1048576

The extra byte is an over-limit sentinel.

The client must:

  1. Inspect Content-Range and Content-Length when present.
  2. Read the response as a stream rather than calling response.text() or building a complete Blob.
  3. Retain at most the limit plus the sentinel byte.
  4. Cancel immediately when the sentinel byte is observed.
  5. Enforce the same limit when the server ignores Range and returns 200.
  6. Render only after end-of-stream proves that the complete object is within the limit.

An over-limit object opens an explanation state with its known size, the 1 MiB policy, and a Download action. It never shows a prefix fragment.

Request identity and cancellation

A preview request is identified by:

bucket + object name + version ID

The request must use the existing generated API client or an equivalent base-path-safe helper so that it preserves:

  • same-origin credentials;
  • the current Console subpath;
  • version_id;
  • anonymous-mode X-Anonymous: 1;
  • current error handling and permission boundaries.

Close, object change, version change, bucket change, and component unmount must abort the active request and clear the old content.

Abort alone is insufficient. A generation token or invalidation flag must also prevent a response that already completed reading or decoding from updating a newer preview.

An aborted request is not an error and must not produce an error toast.

Encoding and fidelity

The first release supports strict UTF-8 only:

new TextDecoder("utf-8", { fatal: true })

Requirements:

  • handle the UTF-8 BOM without displaying it;
  • preserve Unicode text, emoji, tabs, LF, and CRLF;
  • reject invalid UTF-8 rather than inserting replacement characters;
  • reject decoded NUL characters as binary or unsupported content;
  • do not guess another encoding;
  • do not log or persist object text;
  • always retain Download as the original-byte escape hatch.

The unsupported-encoding state should explain:

This object is not valid UTF-8 text or contains binary data. Download it to inspect the original bytes.

JSON and XML are displayed exactly as decoded source text. The first release must not run JSON.parse followed by JSON.stringify: that can alter unsafe integers, duplicate keys, whitespace, lexical forms, and the text users copy.

Safe renderer

The success state renders one text node:

<pre>{content}</pre>

The implementation must not use:

  • iframe, object, or embed;
  • dangerouslySetInnerHTML or innerHTML;
  • DOMParser or an XML parser;
  • Markdown or HTML rendering;
  • an HTML data/blob URL;
  • per-line or per-token spans;
  • automatic links, ANSI escapes, or syntax markup.

One bounded text node keeps the DOM cost predictable and the security property inspectable.

The preformatted region uses a monospace font, preserves whitespace, defaults to no wrapping, owns both scrollbars, is keyboard focusable, and supports native selection and copy. No-wrap is intentional: it preserves aligned logs and avoids expensive layout of a single very long line.

UI states and permissions

The Preview action is enabled only when:

eligible preview type
AND object read permission
AND not a delete marker
AND not a prefix

The object-detail conjunction bug must be fixed, and list and detail surfaces must share the same eligibility function.

An eligible over-limit object still offers Preview. The modal explains why content is not loaded; disabling the button would leave the user unable to distinguish size, permission, and type failures.

The modal distinguishes:

State Required behavior
Loading Accessible busy state; no stale text.
Success Scrollable raw text plus Download.
Empty Explicit “File is empty” state.
Too large Object size, 1 MiB limit, Download; no body request when size is already known.
Invalid UTF-8 / binary Dedicated explanation and Download.
Forbidden Permission-specific message; no retained text.
Not found / replaced Object-change message; no retained text.
Network / server error Actionable retry/download state.
Aborted / closed Silent cleanup.

HTTP error bodies must never be decoded and displayed as object content.

All new user-facing strings go through the existing translation layer and ship in English and Chinese together. The content region and controls must remain usable in light and dark themes and at narrow widths.

Functional and security requirements

Functional requirements

  • FR1: Existing media and PDF classification remains unchanged.
  • FR2: The text fallback follows the normative extension/MIME matrix.
  • FR3: Eligible complete objects up to 1 MiB render as strict UTF-8 source.
  • FR4: Over-limit objects render no partial content.
  • FR5: Empty objects have a distinct successful empty state.
  • FR6: Current and selected historical versions use the same version for metadata, size, and body.
  • FR7: Anonymous access and subpath hosting retain their current request behavior.
  • FR8: List and detail actions apply the same type and permission decision.
  • FR9: Download, share, media, PDF, and storage behavior do not change.

Security requirements

  • SR1: Object bytes can reach the DOM only through text content.
  • SR2: Text Preview contains no document renderer or parser.
  • SR3: At most 1 MiB plus one sentinel byte is retained.
  • SR4: Closing or changing identity invalidates every previous response.
  • SR5: Invalid UTF-8 and NUL content are not shown as faithful text.
  • SR6: Errors, Redux, local storage, logs, and telemetry never retain preview text.
  • SR7: Server authorization remains authoritative for direct requests.
  • SR8: No CSP or backend inline MIME relaxation is introduced.

Implementation scope

Expected Console changes:

  1. Refactor preview classification so the current media decision is preserved and text is an explicit fallback.
  2. Add text to the preview type union.
  3. Add a dedicated PreviewText component with streaming bounds, strict decode, request cancellation, and explicit states.
  4. Route text objects explicitly to that component.
  5. Remove the unreachable generic iframe fallback.
  6. Fix the object-detail Preview disable expression and share eligibility logic with the list surface.
  7. Preserve unknown size instead of coercing it to zero.
  8. Add English and Chinese strings.
  9. Add classification, component, resource, security, permission, version, and browser tests.

Expected unchanged areas:

  • Console and S3 API paths;
  • the backend safeMimeTypes list;
  • Content Security Policy;
  • object storage and metadata formats;
  • image, PDF, audio, video, download, and share handlers;
  • external frontend dependencies.

If a future product requires tailing, server-side transcoding, organization-wide policy, or reliable behavior through proxies that ignore Range, a dedicated server endpoint may be designed separately.

Rejected alternatives

Keep text preview disabled

Benefit: no new code or browser memory use.
Rejected because: logs and configuration objects are a routine object-storage workflow, and download-only inspection is an avoidable Console regression.

Reuse the same-origin iframe

Benefit: minimal code and browser-native presentation.
Rejected because: it turns uploader-controlled content and mutable MIME metadata into a same-origin document boundary. It also leaves resource use unbounded.

Add a backend preview API now

Benefit: central server-side limits and normalized text responses.
Rejected for the first release because: the user already has object-read permission, and the existing download endpoint provides versioning, authorization, and Range. A new API would duplicate contracts without establishing a new data-access boundary.

Show the first 1 MiB of a large object

Benefit: better large-log convenience.
Rejected because: partial JSON/XML is structurally misleading, UTF-8 boundaries need additional handling, and a single “preview” action would no longer mean complete content.

Decode invalid UTF-8 with replacement characters

Benefit: some damaged or legacy logs remain partially readable.
Rejected because: copied text would no longer faithfully represent the stored object. Lossy viewing and other encodings require a separate, explicit product mode.

Auto-format JSON

Benefit: more readable indentation.
Rejected because: parse/stringify can alter numbers, duplicate keys, lexical representation, and copied content. A future opt-in formatted view may sit beside, never replace, the raw default.

Add Monaco or another code editor

Benefit: line numbers, search, highlighting, and folding.
Rejected because: bundle, worker, CSP, and maintenance costs exceed the needs of a bounded read-only preview. A native <pre> is smaller and easier to audit.

Acceptance and test plan

Classification matrix

Automated tests must lock every normative matrix row, extension case handling, MIME parameter stripping, explicit HTML/XHTML denial, and unchanged media conflicts.

Resource tests

Cover:

  • 0 bytes;
  • 1 byte;
  • exactly 1,048,576 bytes;
  • 1,048,577 bytes;
  • known over-limit size with zero body requests;
  • unknown size;
  • 206 with a revealing Content-Range;
  • server ignores Range and returns 200;
  • missing or false Content-Length;
  • close and identity changes during streaming.

No case may retain or render more than the complete allowed object.

Encoding and fidelity tests

Cover UTF-8 Chinese, emoji, tabs, LF, CRLF, BOM, invalid byte sequences, NUL bytes, JSON unsafe integers, duplicate keys, original whitespace, XML declarations, DOCTYPE, CDATA, and stylesheet processing instructions.

The raw success view must preserve decoded text. Invalid and binary cases must show their dedicated state.

Security tests

Payloads containing <script>, event attributes, iframe tags, SVG handlers, XML stylesheets, external entities, and suspicious URLs must:

  • appear literally in <pre>.textContent;
  • create no corresponding DOM elements;
  • execute no script or dialog;
  • cause no object-content-originated request;
  • encounter no iframe, object, embed, HTML parser, or XML parser in Text Preview.

Permission and race tests

Verify:

  • no GetObject means no usable action and no retained body;
  • historical versions require their corresponding permission;
  • metadata and body use the same version ID;
  • a late old response cannot replace a new object’s preview;
  • 401, 403, 404, 416, and 5xx bodies never become preview content;
  • anonymous access and Console subpaths do not regress.

Browser regression

Use a real SILO/Console test instance to inspect both English and Chinese routes, light and dark themes, and narrow and desktop widths. Media, PDF, download, share, and version workflows require smoke coverage alongside the new text states.

Delivery and completion gates

The change belongs to pgsty/silo-console, even though the user report is tracked in the SILO server repository.

Delivery is staged:

  1. Merge the focused Console source and test change.
  2. Pass TypeScript checking, production build, automated matrices, and real-browser security regression.
  3. Update Console release notes and regenerate the actual embedded web assets.
  4. Publish a Console version; a minor release is appropriate for the new visible capability.
  5. Update SILO’s github.com/minio/console => github.com/pgsty/silo-console replacement to the exact new pseudo-version.
  6. Build a SILO candidate from that exact dependency and repeat integration checks.
  7. Publish the SILO binary and image, naming the first version that contains the feature.

These are separate states:

Gate Meaning
Console PR merged Implementation exists in source.
Console assets/tag published Console is independently consumable.
SILO dependency updated SILO main has integrated the change.
SILO release published Users can obtain the feature.

Issue #17 should not be described as fixed for users merely because a local preview or Console source PR exists.

Trade-off summary

The accepted design favors:

  • explicit scope over a generic browser viewer;
  • complete small files over partial large files;
  • source fidelity over automatic formatting;
  • strict UTF-8 over silent lossy decoding;
  • one inert text node over a full editor;
  • the existing download API over a new backend contract;
  • a verifiable security invariant over convenient same-origin rendering.

The cost is real: large logs and legacy encodings still require download, and the first release has no search, line numbers, wrapping toggle, or highlighting. Those omissions are deliberate. They make the feature small enough to audit and strong enough to trust.

Review record

The design was independently reviewed from three perspectives:

  • product scope, delivery, and acceptance;
  • security and frontend architecture;
  • compatibility and current-source verification.

The reviewers initially differed on MIME-only eligibility and lossy UTF-8 fallback. After cross-review they reached a single contract:

  • existing media classification wins;
  • text fallback accepts the four target extensions or four exact normalized MIME types;
  • HTML/XHTML extensions are explicitly excluded;
  • strict UTF-8 and NUL rejection are required;
  • lossy viewing is deferred to a separate proposal.

No unresolved design question remains. Implementation may proceed against this record.

4 - One Endpoint, Two Privileges: Separating User and Group Status

This document records the discussion, repair, and final authorization design for upstream issue minio/minio#21478 and SILO PR #73.

Status on 2026-08-26: SILO PR #73 was merged as 2e2377d1c, preserving the signed-off repair commit 58735ee38. All eight reported checks passed. Upstream issue #21478 and PR #21482 remain open, but minio/minio is archived and read-only, so no further issue comment or merge can be made there.
Group follow-up on 2026-08-28: final release review found the same fixed-action defect in set-group-status. Signed-off server commit d98250110 now selects admin:EnableGroup or admin:DisableGroup from the requested target state and adds a real four-way IAM authorization test. Local verification and independent review are complete; push, remote CI, merge, tag, and delivery remain pending.
Scope: authorize enabling and disabling a user with their respective existing Admin Actions. Do not change the route, status values, account storage, replication record, or client API.
Security property: possessing admin:DisableUser must not grant the ability to enable an account, and possessing admin:EnableUser must not grant the ability to disable one.
Release boundary: merge, tag, release package, container image, deployment, and production verification remain separate gates.

Too Long; Didn’t Read (TL;DR)

SILO exposes both admin:EnableUser and admin:DisableUser, but the shared set-user-status handler historically authorized every request with admin:EnableUser. A policy that granted only admin:DisableUser therefore could not disable an account. The workaround was to grant admin:EnableUser as well, which destroyed the least-privilege boundary that the two action names promised.

The selected repair derives exactly one required action from the requested target state before authorization:

Requested status Required action
enabled admin:EnableUser
disabled admin:DisableUser
invalid or unknown admin:EnableUser, preserving the previous authorization-before-validation default

The handler then calls validateAdminReq once. A four-way IAM test proves both positive operations and both denied cross-action operations. This is intentionally stricter than preserving the accidental historical behavior in which an Enable-only policy could also disable users.

The same rule now applies to group status:

Requested group status Required action
enabled admin:EnableGroup
disabled admin:DisableGroup
invalid or unknown admin:EnableGroup, preserving the previous authorization-before-validation default

Before the follow-up, an EnableGroup-only principal could disable a group, while a DisableGroup-only principal received AccessDenied for that exact operation. The group repair uses the same one-selector, one-authorization design rather than treating the two actions as aliases.

The reported defect

The Admin API uses one route for both state transitions:

PUT /minio/admin/v3/set-user-status
    ?accessKey=<target>
    &status=enabled|disabled

Before the repair, the handler checked one fixed action before reading the requested status:

objectAPI, creds := validateAdminReq(ctx, w, r, policy.EnableUserAdminAction)

The later call to SetUserStatus correctly received either enabled or disabled, but authorization had already treated both as Enable operations. admin:DisableUser existed in the policy vocabulary and documentation while being ineffective for this endpoint on its own.

Issue #21478 supplied the practical counterexample: an operator wanted a policy that could disable accounts during an incident without being able to restore them. A policy containing admin:DisableUser received AccessDenied; adding admin:EnableUser made the request work, but also gave the operator the more powerful recovery transition that the policy intentionally withheld.

This is not a missing convenience permission. It is a mismatch between the policy model and the enforcement point:

policy says:     DisableUser only
request says:    target state = disabled
handler checked: EnableUser
result:          legitimate disable denied
workaround:      grant an unwanted enable capability

Why two actions must mean two capabilities

An account state transition has direction. Disabling is commonly delegated to incident responders, fraud controls, compliance automation, or a break-glass process. Enabling restores access and may require a separate approver.

If either action authorizes both transitions, a policy author cannot express that separation. The server would publish two names while enforcing one combined capability. The design contract is therefore strict:

Principal policy Disable target Enable target
admin:DisableUser only allow deny
admin:EnableUser only deny allow
both actions allow allow
neither action deny deny

The built-in consoleAdmin policy grants admin:*, so full administrators retain both operations. The compatibility impact is limited to custom restricted policies that relied on the old accidental behavior.

The public PBAC reference now states the same contract for admin:EnableUser and admin:DisableUser.

Design goals and non-goals

Goals

  1. Make both existing Admin Actions enforceable according to their names.
  2. Preserve least privilege in both directions.
  3. Perform one authorization decision and write at most one authorization error.
  4. Preserve the route, request values, response format, self-mutation guard, IAM storage call, and site-replication hook.
  5. Encode the contract in tests that fail if the two permissions are broadened or swapped again.

Non-goals

  • split the endpoint into separate enable and disable routes;
  • add a new combined action or change policy syntax;
  • change user status persistence or replication;
  • redesign Console permissions;
  • infer release, image, deployment, or production delivery from a source merge.

Alternatives considered

Keep checking admin:EnableUser for both states

This preserves behavior but leaves admin:DisableUser unusable and forces over-privileged policies. It is the defect, not a compatibility contract worth retaining.

Require both actions for either transition

This makes the two labels decorative and prevents delegated disable-only operation. It is stricter in quantity but weaker in expressiveness and least privilege.

Try Enable authorization, then retry Disable authorization

Upstream PR #21482 attempted this shape for a disabled request. It first called validateAdminReq with EnableUser, then called it again with DisableUser if the first result was nil.

That helper has an important contract: when it returns a nil object layer, it has already written an error response. A Disable-only request can therefore commit a 403 response before the second authorization succeeds and the handler proceeds to mutate account state. Authorization fallback must never continue after an error response has been committed.

Accept either Enable or Disable for a disabled request

validateAdminReq already accepts multiple actions and succeeds if any one is allowed, so compatibility behavior could be implemented safely with one variadic call. That would let Disable-only policies work while preserving the historical ability of Enable-only policies to disable.

SILO rejected this option because the historical ability was the enforcement bug. It would solve the reporter’s positive case but retain a cross-action privilege that contradicts the two-action model. Operators who want both transitions can grant both actions explicitly.

Validate the status before authenticating

Rejecting unknown status values first would change error precedence: a caller that previously had to pass the Enable authorization gate could now receive a validation result before authorization. The repair does not need that broader behavioral change.

Unknown values therefore retain admin:EnableUser as the authorization default. Valid disabled is the only value that selects admin:DisableUser; the existing IAM layer remains responsible for rejecting invalid status values after authorization.

The selected implementation

The repair adds a pure selector:

func setUserStatusAdminAction(status string) policy.AdminAction {
    if madmin.AccountStatus(status) == madmin.AccountDisabled {
        return policy.DisableUserAdminAction
    }
    return policy.EnableUserAdminAction
}

The handler reads the route variables, selects the action, and authorizes exactly once:

vars := mux.Vars(r)
accessKey := vars["accessKey"]
status := vars["status"]

objectAPI, creds := validateAdminReq(ctx, w, r, setUserStatusAdminAction(status))
if objectAPI == nil {
    return
}

Everything after the gate remains unchanged:

  • a caller still cannot enable or disable its own account;
  • globalIAMSys.SetUserStatus validates and persists the requested status;
  • site replication records the same status and timestamp;
  • response and audit behavior use the existing path.

The selector depends only on the requested target state. It does not load the current user, infer a transition from stored state, or make authorization depend on whether the target exists. This keeps authorization deterministic and avoids a read-before-authentication dependency.

Why the repair is safe

The correctness argument consists of five invariants:

  1. Every valid status maps to exactly one Admin Action.
  2. validateAdminReq is invoked once, so a failed authorization cannot be followed by mutation.
  3. The mutation call is reachable only after the selected action succeeds.
  4. Invalid status values preserve the old Enable authorization boundary and are still rejected by the existing status-validation path.
  5. No storage, replication, wire, or client contract changes; only the permission required to reach the existing mutation changes.

The change is a deliberate authorization tightening for Enable-only custom policies that used the disable operation. That tightening is the mechanism that makes admin:DisableUser a real independent capability.

Test design

Pure action mapping

The unit test fixes three selector cases:

Input Expected action
enabled EnableUser
disabled DisableUser
invalid legacy EnableUser default

Four-way IAM authorization matrix

The integration test creates separate users and policies, then exercises the real Admin API:

  1. a Disable-only client successfully disables a target;
  2. the same client receives AccessDenied when enabling it;
  3. an Enable-only client successfully enables the target;
  4. the same client receives AccessDenied when disabling it.

Positive assertions alone would not prove least privilege: both policies could accidentally authorize both states and still pass. The two negative cross-action assertions are the security regression tests.

The test removes every temporary user and policy after execution. It runs inside the existing IAM server suite, so it covers request signing, policy attachment, handler authorization, persistence, and Admin-client error decoding rather than testing only the helper.

Repair and verification record

The server checkout originally contained unrelated dependency, generated-credit, checksum-test, and security-document changes, while local main was behind the remote. The two user-status files were isolated into a clean worktree based on current origin/main; no unrelated file entered the repair commit.

Local verification passed:

go test ./cmd -run '^TestSetUserStatusAdminAction$' -count=1
go test ./cmd -run '^TestIAMInternalIDPServerSuite$' -count=1
git diff --check

The signed-off commit 58735ee38 was pushed in PR #73. Its eight remote checks all passed:

  • DCO sign-off;
  • format, build, and vet;
  • lint and generated files;
  • cmd/ tests;
  • internal/ tests;
  • race detector and S3 Select;
  • cross compilation;
  • vulnerability analysis.

The PR was merged with the repository’s normal merge strategy as 2e2377d1c. Local main was then fast-forwarded only after the two original working files were byte-for-byte and patch-ID identical to the merged result. The unrelated local changes remained intact, and the temporary worktree and task branch were removed after the code became recoverable from main and PR #73.

Least-privilege policy examples

Disable-only operator

{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Allow",
      "Action": [
        "admin:DisableUser",
        "admin:GetUser"
      ]
    }
  ]
}

This principal can inspect and disable another user, but cannot enable it.

Enable-only operator

{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Allow",
      "Action": [
        "admin:EnableUser",
        "admin:GetUser"
      ]
    }
  ]
}

This principal can inspect and enable another user, but cannot disable it. Grant both actions explicitly to roles responsible for the complete account lifecycle.

Group-status follow-up

The group endpoint has the same shape as the user endpoint:

PUT /minio/admin/v3/set-group-status
    ?group=<target>
    &status=enabled|disabled

It also publishes two existing actions, admin:EnableGroup and admin:DisableGroup. The inherited handler nevertheless authorized every request with EnableGroup before reading status. This was not merely a dead permission: it reversed least privilege in both directions. The wrong principal could disable a group, and the intended disable-only principal could not.

The follow-up adds setGroupStatusAdminAction, deliberately matching setUserStatusAdminAction:

func setGroupStatusAdminAction(status string) policy.AdminAction {
    if madmin.GroupStatus(status) == madmin.GroupDisabled {
        return policy.DisableGroupAdminAction
    }
    return policy.EnableGroupAdminAction
}

The integration test creates separate EnableGroup-only and DisableGroup-only administrators and a real target group. It proves:

  1. DisableGroup-only can disable;
  2. DisableGroup-only cannot enable;
  3. EnableGroup-only can enable;
  4. EnableGroup-only cannot disable.

The suite exercises signed Admin requests, policy attachment, handler authorization, IAM mutation, response decoding, and cleanup. Invalid status still selects the legacy Enable action before the existing validation error, so the change does not expose a new pre-authentication oracle. The successful site-replication hook remains after mutation and is not called for denied requests.

This follow-up changes no user behavior and introduces no new policy action. It makes the two already documented group actions enforce the same state-specific contract as their user counterparts.

Compatibility and migration

No client or API migration is required. The endpoint, query parameters, status strings, success response, and Admin-client method are unchanged.

Policy review is required for restricted administrative roles:

  • a role that should only disable users needs admin:DisableUser;
  • a role that should only enable users needs admin:EnableUser;
  • a role that must do both needs both actions;
  • consoleAdmin and other admin:* policies are unaffected;
  • a legacy custom policy containing only admin:EnableUser can no longer use that permission to disable users and must add admin:DisableUser if both operations are intended.

The equivalent rules now apply to group-management roles:

  • a role that should only disable groups needs admin:DisableGroup;
  • a role that should only enable groups needs admin:EnableGroup;
  • a role that must do both needs both actions;
  • a legacy EnableGroup-only role can no longer disable groups.

This is a source-level compatibility change in authorization behavior, not a wire-protocol break.

Upstream disposition

As of this record, upstream issue #21478 and PR #21482 are still displayed as open. The upstream repository is archived and read-only. An attempt to leave the single-authorization analysis on the PR was rejected by GitHub because archived, locked discussions cannot accept comments.

The upstream artifacts remain useful provenance but are no longer an actionable delivery path. SILO owns its implemented semantics, tests, merge, release note, and eventual production verification.

Delivery state

Gate User repair Group follow-up on 2026-08-28
Design decision complete complete
Implementation and local tests complete complete
Independent adversarial review complete complete, GO
Signed-off commit complete local d98250110
Push, PR CI, and merge complete not established
Tagged SILO release not established not established
Release package or container image not established not established
Deployment not established not established
Production behavior not established not established
Upstream merge unavailable; repository archived not applicable

Conclusion

The repairs make the authorization model tell the truth. Enabling and disabling users or groups are opposite state transitions with different operational risk, and SILO already exposes different policy actions for each direction. Each handler must therefore select the action from the requested target state and authorize once before mutation.

The code change is small because the design boundary is clear. The durable result is larger: an explicit permission matrix, rejected compatibility alternatives, an invalid-input rule, a four-way integration test, a clean merge record, migration guidance, and an honest release boundary.

5 - Config Environment Files Are Not Shell Scripts

This record defines the startup contract for MINIO_CONFIG_ENV_FILE and explains the compatibility repair committed in SILO as ce456dba0.

Status on 2026-08-28: implementation, focused tests, the complete cmd and internal suites, tagged tests, race tests, vet, lint, generated-file checks, rebrand guards, build, and an independent local Fable Max review are complete. The server commit exists locally; push, remote CI, merge, tag, package, image, deployment, and production verification remain separate gates.
Scope: environment-file parsing and named-target discovery only. No configuration key, subsystem, value, precedence, storage format, or client API changes.
Compatibility rule: the file is a SILO input format. Supporting an optional export prefix does not make it a POSIX shell program.

Too Long; Didn’t Read (TL;DR)

SILO can load startup variables from a file:

export MINIO_CONFIG_ENV_FILE=/etc/default/silo
silo server /data

The parser accepts assignments such as:

MINIO_ROOT_USER = silo-admin
MINIO_ROOT_PASSWORD = "  significant surrounding spaces  "
MINIO_NOTIFY_WEBHOOK_ENABLE_my-hook = off
MINIO_NOTIFY_WEBHOOK_ENDPOINT_my-hook = https://events.example.com/minio

The last two names are important. Multi-target configuration appends the target name verbatim after an underscore. The configuration subsystem does not restrict a target to a shell identifier; names containing -, ., :, digits, or printable Unicode can be discovered and resolved exactly.

A hardening change accidentally validated every key as [A-Za-z_][A-Za-z0-9_]*. It made my-hook invalid and stopped the server during restart even though the previous loader and the configuration target model accepted it. The repair validates what SILO actually needs instead:

  • the name is non-empty, valid UTF-8, and made of visible non-whitespace characters;
  • = and NUL are not allowed in a name;
  • NUL is not allowed in a value;
  • invalid input reports file and line without reporting the value;
  • the complete file is parsed before any assignment is applied.

Why the regression was real

The environment-file loader calls os.Setenv after parsing. An operating-system environment is a list of strings, not a shell variable namespace. Shell assignment syntax is narrower because the shell must tokenize and expand variable names in its own language.

Named SILO configuration targets are built differently:

MINIO_<SUBSYSTEM>_<PARAMETER>_<target>

For example:

MINIO_NOTIFY_WEBHOOK_ENABLE_my-hook
MINIO_NOTIFY_WEBHOOK_ENDPOINT_my-hook

Target discovery lists variables by the fixed parameter prefix and treats the remaining suffix as the target name. Target lookup reconstructs the same name without uppercasing or sanitizing that suffix. Rejecting - in the file parser therefore broke a valid discover-to-resolve path; it did not protect a shell evaluation path because no shell evaluates the file.

The failure is operationally sharp. MINIO_CONFIG_ENV_FILE is loaded only at startup. A server can continue running with an old process environment, then fail on its next restart after the file or binary changes. Startup must fail on malformed input, but it must not invent a narrower target grammar than the configuration system.

The file grammar

Lines and comments

  • blank lines are ignored;
  • a line whose first non-whitespace character is # is ignored;
  • an optional standalone export followed by whitespace is removed;
  • exportFOO=value remains the key exportFOO; it is not mistaken for the prefix;
  • the first = separates key and value, so additional = characters remain part of the value.

The file is not a shell. It does not perform variable expansion, command substitution, backslash processing, or inline-comment interpretation.

Keys

Surrounding whitespace around the key is removed. The remaining key must:

  1. be non-empty valid UTF-8;
  2. contain only Unicode graphic characters;
  3. contain no whitespace, =, NUL, control, or invisible format characters.

This preserves OS-compatible names and multi-target suffixes while rejecting visually empty or structurally ambiguous keys. A key beginning with a digit or punctuation is accepted by the parser; SILO still reads only the exact names used by its configuration and runtime components.

Values and quoting

Unquoted values are trimmed. To retain leading or trailing spaces, quote the complete value with matching single or double quotes:

PLAIN = value
SPACED = "  value with significant spaces  "
TOKEN = scheme://user:password@example.com?a=b
EMPTY =

The parser removes one matching outer quote pair. It does not interpret escapes inside the quoted value. NUL is always rejected because it cannot be represented in an environment entry.

Failure and secrecy contract

Syntax errors stop startup. Diagnostics include the file path, line number, and the invalid key or error class, but never the value. A password on a malformed line must not be copied into logs.

Parsing is all-or-nothing: a syntax error returns no entries, and assignment starts only after the complete file has parsed. If the operating system rejects a validated assignment, SILO also stops startup and identifies the key and file. Since the process exits, it never serves requests with a partially loaded environment.

The file itself remains a privileged secret-bearing input. Operators must protect it with appropriate ownership and mode; parser validation is not a substitute for filesystem permissions.

Regression matrix

The committed tests cover:

  • spaces and tabs around =;
  • quoted values with significant spaces;
  • standalone export, including Unicode whitespace after it;
  • keys beginning with _, a digit, or punctuation;
  • named targets using -, ., :, and Unicode;
  • exact named-target discovery through the configuration subsystem;
  • empty keys, whitespace, NUL, and invisible format characters;
  • NUL values;
  • multiple = characters in URLs and tokens;
  • file-and-line diagnostics that redact values;
  • all-or-nothing parse results.

The implementation passed the complete local server verification matrix and a read-only adversarial review. Windows-specific os.Setenv behavior has not been exercised on a Windows runner; unsupported platform rejection remains fail-fast rather than silent.

Compatibility and delivery

No configuration migration is required. Existing ordinary environment names behave unchanged. Files using shell-style whitespace become more predictable, and previously accepted named targets work again.

The visible compatibility changes are intentional:

  • invalid or invisible names now fail instead of being silently ignored;
  • unquoted surrounding value whitespace is trimmed; quote it when significant;
  • malformed input stops startup with a redacted location-aware error;
  • a valid punctuation-bearing target is no longer rejected merely because a shell could not assign it with NAME=value syntax.

This record describes a source commit, not a delivered release. Until the commit is pushed, tested remotely, merged, tagged, packaged, imaged, and deployed, operators must not assume a public SILO binary contains this parser contract.

Conclusion

Configuration compatibility depends on validating the format SILO actually consumes. MINIO_CONFIG_ENV_FILE borrows a small amount of dotenv-like syntax for operator convenience, but it is not executed by a shell. The repair restores named-target compatibility while retaining strict NUL, invisibility, redaction, and fail-fast guarantees.

6 - Two SSE-C Keys, One CopyObject Response

This record explains the CopyObject SSE-C checksum response repair committed in SILO as e37b0134a.

Status on 2026-08-28: implementation, encryption and key-rotation tests, complete server suites, race tests, static checks, build, and independent Fable Max acceptance review are complete. The commit exists locally; push, remote CI, merge, tag, package, image, deployment, and production verification remain separate gates.
Scope: the successful CopyObject XML and HTTP response after the destination object has committed. Stored object bytes, checksum metadata, encryption format, source decryption, federation, replication, and historical objects are unchanged.
Security property: source SSE-C headers may decrypt only source state; destination SSE-C headers may decrypt only committed destination state.

Too Long; Didn’t Read (TL;DR)

An SSE-C copy can use two independent keys:

Role Request headers Purpose
source X-Amz-Copy-Source-Server-Side-Encryption-Customer-* decrypt the source object
destination X-Amz-Server-Side-Encryption-Customer-* encrypt and later interpret the committed destination object

SILO correctly wrote the destination with its destination key. However, after commit, both the XML generator and the generic PUT-response header helper received the complete CopyObject request. The checksum metadata decrypter intentionally prefers copy-source SSE-C headers when they are present. That priority is correct while reading the source, but wrong when interpreting the committed destination.

With source key A and destination key B:

source body       --decrypt A--> logical bytes
logical bytes     --encrypt B--> committed destination
destination csum  --seal B-----> stored checksum metadata
response decoder  --try A------> key mismatch, checksum omitted

The object and stored checksum were correct; only the successful response was incomplete. The repair constructs a destination response-header view by removing exactly the three copy-source SSE-C customer headers. It decrypts the destination checksum once, then reuses the resulting map for both XML and HTTP response headers.

Observable failure

The failure requires a checksum-bearing destination and distinct source/destination SSE-C contexts. A representative request supplies:

x-amz-copy-source: /bucket/source
x-amz-copy-source-server-side-encryption-customer-algorithm: AES256
x-amz-copy-source-server-side-encryption-customer-key: <key A>
x-amz-copy-source-server-side-encryption-customer-key-md5: <md5 A>
x-amz-server-side-encryption-customer-algorithm: AES256
x-amz-server-side-encryption-customer-key: <key B>
x-amz-server-side-encryption-customer-key-md5: <md5 B>
x-amz-checksum-algorithm: CRC32

Before the repair:

  • CopyObject returned HTTP 200;
  • reading the destination with key B returned the correct body;
  • stored destination checksum metadata decrypted with key B and matched the logical bytes;
  • the CopyObject XML and HTTP response omitted CRC32 and ChecksumType.

This is a response-contract defect, not evidence of corrupted object data.

The same ambiguity affects same-object SSE-C key rotation. After metadata has been resealed under key B, the request still carries source key A in the copy-source headers. Response generation must describe the post-rotation object, so it must use B.

Why the global decrypter must not change

The metadata decrypter’s copy-source priority is not itself a bug. Earlier in CopyObject, the server examines source checksum metadata to decide whether to preserve its algorithm, recompute a full-object value, or add the default CRC64NVME checksum. For an SSE-C source, that metadata is protected by the source object key and therefore requires the copy-source headers.

Changing the global priority to prefer destination SSE-C headers would fix the final response while breaking source checksum interpretation. The safe boundary is temporal and object-specific:

before commit: full request headers, source context
after commit:  destination-only SSE-C headers, destination context

The repair applies only at that post-commit boundary.

Selected implementation

Destination response view

The handler clones the request headers and removes exactly:

  • X-Amz-Copy-Source-Server-Side-Encryption-Customer-Algorithm;
  • X-Amz-Copy-Source-Server-Side-Encryption-Customer-Key;
  • X-Amz-Copy-Source-Server-Side-Encryption-Customer-Key-MD5.

Regular destination SSE-C headers remain. SSE-S3 and SSE-KMS destination metadata needs no customer key and continues through the existing path.

Decrypt once, project twice

Before the repair, CopyObject called decryptChecksums once while building XML and again while writing success headers. For SSE-S3 or SSE-KMS this could repeat KMS unseal work.

The repaired flow is:

committed ObjectInfo
  -> decryptChecksums(destination headers) once
  -> CopyObjectResult XML fields
  -> x-amz-checksum-* and x-amz-checksum-type response headers

The generic setPutObjHeaders wrapper remains available to PutObject, CompleteMultipartUpload, and DeleteObject. CopyObject calls a narrow helper that accepts the already decrypted checksum map. ETag, VersionID, delete-marker, lifecycle prediction, and checksum header behavior remain in one shared implementation.

Regression matrix

The tests cover:

  • plaintext source to SSE-C destination;
  • compressed and uncompressed SSE-C destinations;
  • SSE-C source key A to destination key B;
  • checksum value and type in both CopyObject XML and HTTP headers;
  • stored checksum decrypted with destination key B;
  • destination body readable with B;
  • same-object key rotation from A to B;
  • checksum response after rotation;
  • SSE-S3 source and destination combinations;
  • all object-layer backends used by the API test harness.

The final combined tree passed focused encryption tests, the complete cmd and internal suites, the project’s tagged test configuration, full go test -race ./..., vet, lint, generated-file checks, rebrand guards, and a local build. A mirror Fable Max review reported no P0–P2 findings and independently confirmed that source decryption still receives the full request while destination response decryption receives the filtered view.

Compatibility and operational impact

  • Successful CopyObject responses: checksum fields that were previously missing now appear when the committed destination has a checksum.
  • Stored objects: no rewrite, migration, metadata-format, or encryption-format change.
  • Existing objects: unaffected; the defect existed only in the one-time successful response.
  • Clients: no request change. Clients already providing both source and destination SSE-C keys receive a more complete S3-compatible result.
  • Performance: one metadata checksum decryption instead of two; no additional object read or hash pass.
  • Rolling upgrade: old nodes may omit the fields while new nodes return them. Stored objects remain mutually readable.
  • Rollback: restores response omission but does not damage objects created while the repair was present.
  • Security: no key or digest value is added to logs or error responses. The response carries only the checksum already authorized for the successful write.

This repair does not resolve the separately deferred legacy federation CopyObject branch and does not audit or modify historical compressed-object checksums. Those questions have different data and operational boundaries.

Conclusion

CopyObject is one request with two object identities. Reusing the full request after commit erased that distinction: a source key was allowed to shadow the destination key while describing destination metadata. The durable repair is not a new encryption scheme; it is an explicit context boundary, followed by one decryption and two faithful response projections.

7 - Why CompleteMultipartUpload Must Return ChecksumType: Review of PR #57

This is the design, review, and decision record for SILO #47 and PR #57.

Status on 2026-08-26: PR #57 was approved and merged as a96116b1; #47 closed automatically. All nine checks on the tested PR head passed, followed by green Go CI and VulnCheck runs on main. No tagged release, package, container image, deployment, or production endpoint has yet been verified to contain the fix.
Scope: return the already-known checksum type from CompleteMultipartUploadResult; do not add new checksum algorithms.
Owner: pgsty/silo, the SILO server repository.
Release boundary: code review, merge, a green main, a tagged release, packages, container images, deployment, and production verification are separate gates.

Too Long; Didn’t Read (TL;DR)

SILO already computed and persisted the correct checksum type for a completed multipart object. HEAD, ListParts, and GetObjectAttributes could expose it. The completion response could not, because its Go response struct had checksum value fields but no ChecksumType field.

PR #57 adds that field, copies the existing value from the checksum map, registers the new exported symbol in the compatibility baseline, and tests FULL_OBJECT, COMPOSITE, and the no-checksum case. It does not recalculate data, change metadata, migrate objects, or weaken integrity checks.

The repair is correct and intentionally narrow. Maintainers approved the fork workflows, refreshed the stale PR branch onto current main, required every new check to pass, submitted an approving review, and merged while preserving the contributor’s signed-off commit. Repository integration is complete; release delivery remains a separate gate.

Where the defect came from

The defect was found while investigating #31, where a real boto3 client exposed several adjacent multipart-checksum incompatibilities. #31 was the data-path failure: a FULL_OBJECT CRC32 multipart upload could fail at completion. It was fixed independently by 0cff48f6c and 75859690b, then closed on 2026-08-04. That review deliberately split four adjacent findings into #46, #47, #48, and #50 instead of treating them as one checksum bug.

After the object completed successfully, another inconsistency remained:

complete_multipart_upload() -> ChecksumType: None
head_object()               -> ChecksumType: FULL_OBJECT

AWS S3 returned FULL_OBJECT in both places. SILO returned the checksum value in the completion XML, and the committed object retained the correct type, but the completion SDK result exposed a null type.

That observation became #47. It is a presentation defect, not a checksum-calculation or storage defect. It does not explain the earlier InvalidPart failure from #31, and repairing it does not replace the server-side part-checksum work tracked in #46, which later landed independently as 7fea6d5a5.

The S3 response contract

The AWS CompleteMultipartUpload API defines ChecksumType as an element of CompleteMultipartUploadResult. Its valid values are:

Value Meaning
FULL_OBJECT The reported checksum covers the logical bytes of the completed object.
COMPOSITE The object checksum is derived from the checksums of its multipart parts.

When an object has no additional S3 checksum, the element should be absent. A server must not invent a type with no checksum value.

This distinction matters to clients. The same Base64 field name can describe either a direct full-object checksum or a multipart composition. A client that validates the completion result needs the type to interpret the checksum correctly and to compare the response with the mode selected at CreateMultipartUpload.

What SILO did before the PR

The completion handler already passed the committed ObjectInfo to generateCompleteMultipartUploadResponse. That generator already called:

cs, _ := oi.decryptChecksums(0, h)

The checksum decoder returned a map containing both the algorithm value and the normalized object type:

CRC32                 -> "...Base64..."
x-amz-checksum-type    -> "FULL_OBJECT" or "COMPOSITE"

The response struct copied the values for CRC32, CRC32C, CRC64NVME, SHA1, and SHA256. It simply had nowhere to put the type:

committed ObjectInfo.Checksum
        -> decryptChecksums
        -> checksum values + x-amz-checksum-type
        -> CompleteMultipartUploadResponse
        -> checksum values copied, type discarded
        -> XML without <ChecksumType>
        -> SDK returns None / null

Other surfaces used the same state correctly. ListParts and GetObjectAttributes already returned ChecksumType; HEAD also reported the stored type. The loss was isolated to the success XML for CompleteMultipartUpload.

What PR #57 changes

The contributed diff contains one signed-off commit, three files, 60 added lines, and no deletions. Only two production lines change. A maintainer later merged current main into the contributor branch to refresh its CI context; that merge changed history, not the three-file product diff.

Add the response field

ChecksumType string `xml:"ChecksumType,omitempty"`

omitempty is part of the compatibility contract: checksum-free uploads retain the old XML shape.

Copy the existing normalized value

ChecksumType: cs[xhttp.AmzChecksumType],

The generator does not infer the type from an ETag, algorithm name, or part count. It uses the same decoded metadata that already supplies the checksum values.

Test the response surface

The added test covers:

  • no checksum: the Go field is empty and <ChecksumType> is absent;
  • a full-object checksum: the field is FULL_OBJECT and the tag is present;
  • a multipart composite checksum: the field is COMPOSITE and the tag is present.

It checks the response value before XML encoding and separately checks omission/presence after encoding.

Record the exported compatibility symbol

CompleteMultipartUploadResponse.ChecksumType is an exported Go field. SILO’s rebrand guard performs an exact comparison of the exported compatibility surface, so the PR correctly adds the field to buildscripts/rebrand-guard/compat-baseline.json. This is an acknowledgement of an intentional public surface change, not a bypass of the guard.

Why the repair works

The correctness argument is a short chain of existing invariants.

  1. ObjectInfo.Checksum is the committed checksum metadata. The completion response is generated only after the object layer returns the committed ObjectInfo.
  2. decryptChecksums(0, h) uses the existing metadata-decryption path, including the request headers needed for SSE-C. No second decryption mechanism is added.
  3. The checksum decoder writes x-amz-checksum-type only when it has decoded a non-empty checksum value.
  4. Existing ChecksumType.ObjType() logic normalizes reachable states to FULL_OBJECT or COMPOSITE.
  5. Indexing a nil or missing map entry returns the empty string.
  6. XML omitempty removes the element for that empty string.

The resulting behavior is deterministic:

Committed checksum state Map value Completion XML
No additional checksum empty no <ChecksumType>
Full-object checksum FULL_OBJECT <ChecksumType>FULL_OBJECT</ChecksumType>
Multipart composite checksum COMPOSITE <ChecksumType>COMPOSITE</ChecksumType>

The change is therefore a missing projection from established state to the wire response. It does not create new checksum state and cannot make an incorrect checksum correct. It makes the response describe the state the server has already validated and committed.

Review and verification

The PR was reviewed after the contributor branch was refreshed onto current main. The update produced head c4b9d38d; the resulting tree hash, 39ec44c6b390c441413e490370f70fbacc4e6a91, exactly matched the isolated local no-commit merge. The result was clean and included the intervening checksum work on main.

Local verification on that exact merge result included:

targeted ChecksumType regression test
CGO_ENABLED=0 go test ./cmd/ -count=1 -timeout 30m
go vet ./cmd/
gofmt and git diff --check
rebrand compatibility guard
local DCO rule

The targeted regression completed in 2.174 seconds and the full cmd package test completed in 168.956 seconds. The commit author email matches its Signed-off-by trailer. Cryptographic Git commit signing is independent of DCO and is not required by this repository.

A separate read-only local Claude Code adversarial review inspected the merged diff, checksum serialization, XML path, current main, tests, DCO, and compatibility guard. Its verdict was COMMENT: the production change was correct and safe, but it preferred an additional HTTP-level completion test before merge. The maintainer agreed that such a test would improve fidelity, but disagreed that it was blocking: the handler delegates directly to the tested generator, while existing real MPU tests already cover persisted FULL_OBJECT and COMPOSITE states. The formal GitHub review therefore recorded APPROVED with the HTTP-level test as a follow-up.

Actions, branch refresh, and merge

The first four action_required runs had been created on 2026-08-09 against the PR’s old base. After approval, DCO passed but the old VulnCheck run used Go 1.26.5 and failed on newly published standard-library vulnerabilities fixed in Go 1.26.6. Current main had already moved to Go 1.27.0, and its latest VulnCheck was green. Treating the stale failure as either a product regression or an ignorable red check would both have been wrong.

The decision was to refresh the test context, not rerun or waive the stale result:

  1. GitHub’s update-branch API merged current main (8d76a255c) into contributor head d014a12cf, producing c4b9d38d without conflicts.
  2. GitHub created four new fork workflow runs for the refreshed head; all four were explicitly approved again.
  3. All nine reported checks passed: DCO, VulnCheck, six jobs in Go CI, and the Test Release Pipeline. The release validation job completed in 11 minutes 26 seconds.
  4. A formal approving review was submitted against c4b9d38d.
  5. Merge used an expected-head guard and the repository’s normal merge strategy, producing a96116b1. This preserved the contributor’s signed-off commit rather than rewriting it through a squash. The PR’s Resolves #47 relationship closed the issue one second later.
  6. The post-merge main VulnCheck and all six Go CI jobs also passed; cross-compilation, the slowest job, completed in 9 minutes 54 seconds.

This sequence matters because “the patch passed once” was not the acceptance criterion. The exact tree merged into current main had to be the tree reviewed and tested, and a stale CI environment could not substitute for that proof.

Evaluation of the PR

What is strong

  • The scope matches the defect. Two production lines restore one missing response element.
  • It reuses authoritative state. There is no duplicate type derivation and no new checksum algorithm branch.
  • Backward compatibility is explicit. omitempty preserves checksum-free responses.
  • The test covers both valid values and absence. A regression cannot silently restore the null result.
  • The compatibility baseline is updated deliberately. CI is not weakened.
  • DCO provenance is complete. The sole commit has a matching sign-off.

Non-blocking review notes

The test is correct for the changed generator but its fixtures are not byte-for-byte models of every production multipart metadata flag:

  • the FULL_OBJECT fixture reaches the right value through a non-multipart checksum state rather than a completed multipart state carrying ChecksumMultipart, ChecksumIncludesMultipart, and ChecksumFullObject;
  • the COMPOSITE fixture carries the multipart flag but omits the persisted per-part checksum block.

Existing API-level tests already exercise genuine FULL_OBJECT and COMPOSITE completion and verify their committed types. PR #57 tests the remaining projection from decoded state to the response field and XML. Adding an assertion to those full API tests would improve test fidelity, but it is not required for this two-line repair.

The PR places ChecksumType before the algorithm-specific fields, while AWS’s example response and SILO’s newer CopyObjectResponse place it after them. Mainstream S3 SDKs parse XML by element name, so this is a parity and style detail rather than a compatibility blocker. Moving the field is optional.

Finally, the contributor commit title says feat: even though the PR correctly marks itself as a bug fix. The final merge preserved that signed-off commit instead of rewriting it. This is a history/style imperfection, not a protocol or release blocker.

Why new algorithms do not belong in this PR

AWS now documents additional fields such as SHA512, MD5, and XXHASH variants. Adding those XML fields alone would create false compatibility.

SILO’s current checksum implementation supports CRC32, CRC32C, CRC64NVME, SHA1, and SHA256. A real new algorithm requires coordinated support across:

  • request header parsing and validation;
  • streaming checksum calculation;
  • multipart FULL_OBJECT or COMPOSITE semantics;
  • on-disk checksum encoding and decoding;
  • UploadPart, UploadPartCopy, completion, copy, replication, HEAD, GET, ListParts, and GetObjectAttributes;
  • SDK/client interoperability and a full encrypted/compressed/versioned test matrix.

PR #57 should not grow response-only placeholders for algorithms the server cannot calculate or persist. Each new algorithm family needs a separate compatibility decision, implementation, and review.

Compatibility and operational impact

  • S3 clients: checksum-aware clients receive ChecksumType from future successful multipart completions instead of null.
  • Wire format: one additive XML element appears only when an additional checksum exists. Clients that ignore unknown elements remain unaffected.
  • Integrity: no checksum is recalculated or accepted differently. Existing validation semantics are unchanged.
  • Stored data: no object, part, metadata, or erasure format changes. No migration or backfill.
  • Existing objects: object state remains correct. A past completion response cannot be replayed; use HEAD or GetObjectAttributes to inspect an existing object’s type.
  • Encryption: the response uses the established checksum metadata-decryption path. No key material or new secret is exposed.
  • Performance: one map lookup and one optional XML element; no extra object read, hashing pass, or allocation proportional to object size.
  • Rolling upgrade: old nodes omit the element and new nodes return it. Requests and stored objects remain compatible, but client-visible behavior stabilizes only after all serving nodes are upgraded.
  • Rollback: rolling back removes the response element from future completions; it does not damage objects created while the fix was present.
  • Other repositories: no server dependency, silo-pkg, MCLI, or Console change is required. Public documentation belongs in this site.

This is an additive compatibility repair, not a release feature that requires operators to rewrite data. Its only externally visible effect is a more complete success response.

Merge and release decision

The final decision had six parts:

  1. accept the narrow projection fix without recalculating checksums or changing storage;
  2. keep SHA512, MD5, and XXHASH families out of #57 until they have end-to-end server support;
  3. record an HTTP-level completion test as useful follow-up work, not a blocker for the directly tested generator repair;
  4. reject stale CI as merge evidence, update the branch to current main, and approve the newly created workflows;
  5. merge only after the refreshed head was formally approved and every check was green, using an expected-head guard and a normal merge that preserved the DCO-signed contribution;
  6. let Resolves #47 close the issue, then verify the resulting main workflows independently.

No dependency update, storage migration, or cross-repository implementation was required. That decision is now complete at the repository-integration gate.

A green main still does not prove that a SILO tag, release package, container image, deployment, or production endpoint contains the repair. Those delivery gates remain unverified and must be recorded separately when the next release ships.

Conclusion

PR #57 is a good example of a small compatibility fix whose correctness comes from respecting an existing source of truth. The checksum type was already calculated, validated, persisted, decryptable, and visible through other APIs. The completion response simply failed to project it into XML.

The accepted repair does exactly that projection and nothing more. It makes the wire response honest without touching user data, checksum mathematics, storage layout, or algorithm scope. The fork workflows, refreshed-head review, merge, automatic issue closure, and post-merge main verification are complete. What remains is delivery discipline: distinguish this merged fix from a tagged, packaged, imaged, deployed, and production-verified release.

8 - When the Total Is Unknown: Folder Download Progress

PRD for replacing NaN% with truthful indeterminate progress when SILO Console downloads a streamed folder ZIP, without changing the server API or ordinary file downloads.

Status: Implemented and verified locally; commit, Console release, and Silo dependency update pending · Priority: P1 · Owner: pgsty/silo-console · Related issue: pgsty/silo#62 · PRD review: Claude Fable 5 (xhigh) — APPROVE · Implementation review: Claude Fable 5 (xhigh), 2026-08-23 — APPROVE, no P0/P1/P2 findings

SILO Console shows NaN% in Downloads / Uploads while downloading a folder. The ZIP normally keeps streaming and the stored objects are intact, but the progress bar has crossed from “unknown” into an invalid determinate state. Users see a full-looking bar, assume the transfer failed or finished, and retry it.

The proposed repair is intentionally narrow:

A download may enter determinate mode only when it has a finite, positive total measured in bytes applicable to that response. Without such a total, it remains indeterminate until completion, failure, or cancellation.

The server keeps streaming ZIPs. Ordinary files keep their percentages. The frontend gains one safe calculation boundary, reuses its existing indeterminate renderer, and closes one missing cancellation transition. This record defines why that is both sufficient and the smallest truthful fix.

The observed failure

The defect is present in the current silo-console v2.1.1, which is embedded by Silo RELEASE.2026-08-06T00-00-00Z.

Reproduction:

  1. Put several objects below a prefix such as folder/.
  2. Stay in the parent listing, select folder/, and click Download.
  3. Open Downloads / Uploads before the transfer finishes.
  4. The row displays NaN%; the ZIP request continues.

The runtime check used a prefix containing about 88.7 MiB and throttled Chromium to preserve the observation window. Two independent downloads produced the same NaN% state.

This is a frontend correctness bug. It is not evidence of corrupted objects, an altered disk format, or a failed S3 GET.

What is actually happening

The visible NaN% is the end of a contract mismatch across three layers.

A prefix has no object size

S3 folders are common prefixes, not stored directory objects. In the listing model, a prefix ends in / and carries size=0. The Console already renders that size as -, correctly treating it as not applicable.

The generated API model marks size as omitempty, so logical zeroes are absent from listing JSON. The single-selection thunk nevertheless passes object.size straight into the download helper: a prefix or zero-byte object therefore supplies undefined at runtime (while synthetic prefix records may supply 0). Neither value is a valid denominator.

A streamed ZIP has no known wire length

The server recognizes the trailing /, recursively lists the objects, then connects a zip.Writer to an io.Pipe. Objects are read, deflated, and copied to the HTTP response as the archive is produced.

That behavior is desirable: the server can send the first bytes without holding the complete archive in memory or on disk. Its consequence is equally deliberate: the final compressed byte length does not exist when headers are sent, so the response has Content-Type: application/zip and a filename, but no Content-Length.

The sum of source object sizes is not a substitute. Source sizes are uncompressed bytes; ProgressEvent.loaded counts response bytes after ZIP compression and framing. They are different units.

A progress event does not imply a computable percentage

The client currently computes every event as:

Math.round((event.loaded / fileSize) * 100)

For a prefix, the denominator is zero or absent. Depending on the value and event, JavaScript produces NaN (loaded / undefined or 0 / 0) or Infinity (positive bytes divided by zero).

The progress callback then writes that non-finite value into Redux and sets waitingForFile=false. That second operation is the decisive state error: the task leaves the existing indeterminate branch merely because an event arrived, not because the event contained a usable total. The determinate progress component receives the invalid value and renders an invalid label.

The complete chain is:

common prefix: size = 0
        |
        v
download(..., fileSize = 0)
        |
        v
streamed deflated ZIP, no Content-Length
        |
        v
event.loaded / 0 => NaN or Infinity
        |
        v
invalid percentage enters Redux; waitingForFile becomes false
        |
        v
determinate ProgressBar renders NaN%

Ordinary non-empty files avoid the defect because the server can stat the object, sets Content-Length, and the list size is positive. If the browser emits a progress event for an empty response, a zero-byte file reaches the same arithmetic boundary as a prefix even though it is a real object; it therefore belongs in the regression contract.

Product contract

The UI needs one honest distinction:

  • Determinate means both transferred bytes and total bytes are known in the same unit.
  • Indeterminate means the request is active but the total is unknown.

This yields four load-bearing invariants:

determinate  => total is finite and total > 0
determinate  => percentage is finite and 0 <= percentage <= 100
unknown total => indeterminate
terminal state => not indeterminate

These invariants are more general than objectPath.endsWith("/"): they cover prefixes, zero-byte files, malformed metadata, and any future unknown-length response without inventing object-type exceptions.

Goals and non-goals

Goals

  1. A folder download never displays NaN%, Infinity%, or a fabricated percentage.
  2. Unknown-length transfers use the existing indeterminate animation.
  3. Known-length ordinary files retain their current percentage behavior.
  4. Completion, failure, and cancellation always leave indeterminate mode.
  5. A zero-byte file never produces a non-finite percentage and still reaches success.
  6. No non-finite or out-of-range download percentage enters Redux.
  7. The fix can ship in Console first and then be consumed by Silo as a dependency update.

Non-goals

  • Do not pre-generate or buffer a complete ZIP on the server.
  • Do not use the sum of uncompressed object sizes as network progress.
  • Do not redesign the entire Object Manager state model.
  • Do not route folders through the current immediately-completing BrowserDownload path.
  • Do not solve the browser memory cost of XMLHttpRequest.responseType="blob" here.
  • Do not change whether a cancelled row remains visible until the user clears it.
  • Do not redesign mid-stream ZIP error signaling after HTTP headers have been sent.
  • Do not modify the S3 API, Console API, object layout, or archive contents.

Those are legitimate follow-ups, but coupling them to this defect would enlarge risk without being necessary to restore truthful progress.

The decision

The minimum production repair has four parts.

D1. Calculate only from a valid total

Add a small pure function, separate from DOM and Redux side effects:

type DownloadProgressEvent = Pick<
  ProgressEvent,
  "loaded" | "lengthComputable" | "total"
>;

export const calculateDownloadPercent = (
  event: DownloadProgressEvent,
  objectSize: number,
): number | null => {
  let total: number | null = null;

  if (Number.isFinite(objectSize) && objectSize > 0) {
    total = objectSize;
  } else if (
    event.lengthComputable &&
    Number.isFinite(event.total) &&
    event.total > 0
  ) {
    total = event.total;
  }

  if (
    total === null ||
    !Number.isFinite(event.loaded) ||
    event.loaded < 0
  ) {
    return null;
  }

  return Math.min(
    100,
    Math.max(0, Math.round((event.loaded / total) * 100)),
  );
};

The source priority preserves compatibility:

  1. A finite positive objectSize retains the current ordinary-file calculation.
  2. If object size is unavailable but the browser declares the response length computable and supplies a finite positive event.total, use it.
  3. Otherwise return null: no truthful percentage exists yet.

The helper’s output contract is complete: either null, or a finite number in [0,100].

D2. Keep unknown totals indeterminate

Change the XHR handler to dispatch only a real percentage:

req.addEventListener("progress", (event) => {
  const percent = calculateDownloadPercent(event, fileSize);

  if (percent !== null) {
    progressCallback(percent);
  }

  // No valid total: preserve waitingForFile=true so the existing UI remains
  // indeterminate instead of manufacturing a determinate value.
});

Download rows already start with waitingForFile=true, and ObjectHandled already renders that state with variant="indeterminate". There is no need to widen Redux to number | null, add another boolean, or change MDS.

When the first valid percentage arrives, the existing updateProgress action stores it and sets waitingForFile=false. When no valid percentage ever arrives, the row remains indeterminate until a terminal action.

D3. Make cancellation terminal

Completion and failure already clear waitingForFile. Cancellation does not. Add the missing transition in cancelObjectInList:

item.waitingForFile = false;

Without that line, the repaired prefix download would remain in the indeterminate rendering branch after abort, masking the Cancelled state. The row continues to follow the current product behavior: it remains as a cancelled record and can be removed manually. Automatic removal is not part of this change.

There is one event-order guard at the XHR boundary as well. abort() first produces readystatechange(DONE, status=0) and only then the abort event; without a status-zero return, the generic DONE branch marks the request failed before onabort can mark it cancelled. DONE/status zero is therefore left to the dedicated onerror or onabort handler, and onabort removes the stored request reference.

D4. Normalize an omitted zero-byte size

The single-selection thunk passes object.size || 0, matching the other download entry point. This restores the API model’s omitted logical zero before the helper checks Blob.size === fileSize, so an HTTP 200 zero-byte object completes at 100% instead of being reported as incomplete.

D5. Keep the server stream unchanged

The folder handler continues to generate a deflated ZIP through io.Pipe and omit Content-Length. No API, archive, storage, or resource-management contract changes.

State machine

State waitingForFile percentage Terminal flag Rendering
Queued / no valid progress yet true 0 none indeterminate
Unknown-total transfer true 0 none indeterminate
Known-total transfer false 0..100 none determinate percentage
Completed false 100 done=true success
Failed false last value failed=true, done=true error
Cancelled false 0 cancelled=true, done=true cancelled

The state does not move back from determinate to indeterminate. If a later event lacks a valid total after a valid percentage was observed, the handler simply retains the last valid value.

Failed and Cancelled both set done=true in the existing reducers. ObjectHandled uses done to change its close button from “abort request” to “remove record”; this repair preserves that behavior. The cancelled Redux value remains 0, while the existing ProgressBarWrapper renders a full orange terminal bar with a Cancelled label because ready=true. That established presentation is not part of this repair.

waitingForFile is not the ideal long-term name for “no computable progress.” Renaming it or replacing the booleans with a discriminated union would improve the model, but that is a separate refactor. In this repair, the field already expresses and renders the required state, so reusing it minimizes compatibility risk.

Why this is sufficient

The repair closes the bug by cases.

Ordinary non-empty file

objectSize > 0, so the helper uses the same denominator as today. The result is finite and clamped, updateProgress enters determinate mode, and completion still sets 100%.

Current streamed folder

objectSize is normalized to 0, while lengthComputable=false and event.total=0. The helper returns null; no invalid action is dispatched, so the row remains indeterminate. Completion sets waitingForFile=false, percentage=100, and done=true.

Future response with a real length

If a proxy or later server implementation provides a trustworthy response total, lengthComputable=true and event.total>0. The same code automatically produces a real percentage without another product change.

Zero-byte file

The omitted listing size is normalized to zero, and both totals are then zero, so an intermediate percentage is mathematically undefined. The row stays indeterminate for its usually brief lifetime; the zero-byte Blob now equals the normalized expected size, and the successful response transitions directly to 100%. 0/0 is never evaluated.

Failure and cancellation

Failure already exits indeterminate. The added cancellation transition does the same on abort. No terminal row can continue to look active merely because its total was unknown.

Mathematically, division occurs only when total belongs to (0, +infinity). The result is then clamped to [0,100]. Therefore neither NaN nor Infinity can cross the calculation boundary into Redux or the determinate renderer.

Rejected alternatives

Buffer the ZIP to obtain Content-Length

The server could generate the complete archive in memory or a temporary file, measure it, and then send it. That would provide an exact wire total, but at the cost of memory or disk pressure, delayed first byte, cleanup complexity, and worse concurrent-download behavior. An observability defect does not justify discarding streaming.

Sum the objects under the prefix

That sum is uncompressed logical data. event.loaded measures compressed response bytes plus ZIP framing. The units differ, so the bar could stop below 100%, exceed 100%, or move according to compression ratio rather than transfer completion. Reject.

Convert invalid progress to 0%

This hides the string but lies about the state: determinate 0% means the total is known and no portion has transferred. Users would still interpret the transfer as stalled. Unknown must remain unknown.

Special-case paths ending in /

That fixes the reported prefix but misses a real zero-byte object, invalid metadata, and other unknown-length responses. The correct boundary is denominator capability, not object type.

Send folders through BrowserDownload

The current large-file path creates an anchor and immediately calls the completion callback after clicking it. It cannot report true completion, console-managed cancellation, or a subsequent HTTP failure. It may be the basis of a later streaming-download design, but today it would replace one lie with another.

Sanitize inside ProgressBar

A generic component guard could be useful defense in depth, but it would leave invalid data in Redux and hide the broken state transition from every other consumer. The primary repair belongs where progress becomes application state.

Introduce percentage: number | null now

A discriminated progress state would be cleaner than the current booleans if the Object Manager were being redesigned. Adding null while retaining waitingForFile, done, failed, and cancelled would instead create more contradictory combinations. Removing the old fields is larger than this bug requires. Reuse the already-rendered indeterminate state now; redesign it separately.

Requirements and acceptance

Functional requirements

  • FR1: An unknown total keeps the task indeterminate.
  • FR2: A finite positive object size preserves ordinary-file percentages.
  • FR3: A finite positive event.total is a fallback only when lengthComputable=true.
  • FR4: Every dispatched percentage is finite and within [0,100].
  • FR5: A zero-byte file never displays non-finite progress and reaches success.
  • FR6: Completion, failure, and cancellation leave indeterminate mode.
  • FR7: Versioned objects, anonymous downloads, previews, and long-filename entry points retain their existing call contract.

Non-functional requirements

  • No new server CPU, memory, disk-buffer, or request cost.
  • No new frontend dependency or build step.
  • No change to the S3 API, Console API, ZIP content, or stored objects.
  • The calculation must be testable without a DOM or live store.
  • TypeScript typecheck and the production frontend build must pass.

Acceptance criteria

  1. While a folder ZIP without Content-Length is active, its row shows an indeterminate animation and no percentage text.
  2. On successful completion, the row reports success/100% and the ZIP can be opened.
  3. A normal non-empty file continues to show finite determinate progress and completes at 100%.
  4. A zero-byte file never shows NaN% or Infinity% and completes successfully.
  5. Cancelling an unknown-total download aborts the request and shows Cancelled, not an active animation.
  6. No download path can place a non-finite or out-of-range percentage in Redux.

Test plan

Pure calculation matrix

Use the existing @playwright/test runner for the pure module rather than adding a test framework. This needs one config-only addition in web-app/playwright.config.ts: a dependency-free unit project, for example with testMatch: /.*\.unit\.ts/. The existing chromium project depends on the auth setup against a live Console at localhost:9090; pure calculation and reducer tests must not be gated by that environment. No new dependency is introduced.

Case loaded objectSize lengthComputable event.total Expected
Ordinary file, halfway 50 100 false 0 50
Common prefix 1024 0 false 0 null
Initial zero over zero 0 0 false 0 null
Response-total fallback 50 0 true 200 25
Zero total is unusable 0 0 true 0 null
Loaded exceeds total 150 100 true 100 100
Invalid object size 10 NaN false 0 null
Omitted zero size 10 undefined false 0 null
Invalid response total 10 0 true Infinity null
Negative loaded -1 100 true 100 null

State tests

Cover the transition contract directly:

  1. A new download starts with waitingForFile=true.
  2. No valid progress action means it remains indeterminate.
  3. Valid progress produces a finite value and waitingForFile=false.
  4. Complete produces done=true, waitingForFile=false, percentage=100.
  5. Failure produces failed=true, done=true, waitingForFile=false.
  6. Cancel produces cancelled=true, done=true, waitingForFile=false, percentage=0.

Browser regression

Use the real Console test instance and Chromium:

  1. Create a temporary bucket with several objects below folder/.
  2. Select the prefix from its parent and start the download.
  3. Apply CDP download throttling so the intermediate state is observable. Throttled runs must raise the default 30-second test timeout with test.setTimeout.
  4. Open Downloads / Uploads and verify that the row exists, has no percentage label, and contains neither NaN% nor Infinity%.
  5. Cancel it and verify the Cancelled terminal state.
  6. Restore network conditions in finally.
  7. Download again without throttling, wait for the browser download, and verify the ZIP.
  8. Repeat the relevant assertions for one ordinary non-empty file and one zero-byte file.
  9. Remove the bucket, objects, downloads, and temporary files in teardown.

The current Playwright project is Chromium-only, so CDP is an acceptable test mechanism. If Firefox or WebKit projects are later enabled, keep the pure and state tests cross-browser and gate only the throttled observation behind the Chromium project.

Implementation boundary

Expected Console changes:

  1. Add downloadProgress.ts containing the pure calculation.
  2. Change Objects/utils.ts to dispatch only a non-null percentage, let status-zero terminal events reach their dedicated handlers, and clean up an aborted request.
  3. Normalize omitted zero sizes in the single-selection thunk.
  4. Change cancelObjectInList to clear waitingForFile.
  5. Add calculation, state, and browser regression coverage using existing dependencies, with a dependency-free unit project in playwright.config.ts.

Expected unchanged code and contracts:

  • The Go folder-download handler and its streaming ZIP.
  • ObjectHandled, ProgressBarWrapper, and MDS.
  • IFileItem.percentage: number and the existing thunk callback types.
  • S3 and Console API routes.
  • Stored object and archive formats.

Delivery and rollback

The fix belongs in pgsty/silo-console, not the Silo server repository where the issue was reported.

Delivery order:

  1. Transfer or cross-reference issue #62 to pgsty/silo-console.
  2. Implement the bounded Console change.
  3. Pass typecheck, production build, pure/state tests, and real browser regression.
  4. Publish a new Console release.
  5. Update Silo’s pinned Console pseudo-version or release dependency.
  6. Build a Silo candidate and repeat folder, ordinary-file, zero-byte, cancel, and ZIP-integrity checks.
  7. Publish Silo and record both affected and fixed versions on the issue.

There is no data migration. If the frontend change regresses, Silo can roll back only the Console dependency; server data and API behavior remain compatible.

Definition of done

  • The calculation returns only null or a finite [0,100] number.
  • Active unknown-total folder downloads render indeterminate.
  • Ordinary files retain determinate progress.
  • Zero-byte files never render invalid progress.
  • Complete, failed, and cancelled rows all leave indeterminate mode.
  • The streamed ZIP and server response contract remain unchanged.
  • Typecheck, production build, and automated regressions pass locally.
  • A Console release is published.
  • Silo updates the Console dependency and passes candidate verification.

Follow-up work

Four adjacent improvements deserve separate design records:

  1. Stream large folder downloads directly to the browser or filesystem instead of holding the full Blob in memory.
  2. Replace the Object Manager’s boolean combination with a discriminated progress/terminal state.
  3. Improve end-to-end integrity and error signaling for ZIP failures after headers have been sent.
  4. Add a generic non-finite-value guard to shared progress components as defense in depth.
  5. Repair the pre-existing Blob JSON error decoder and request-trace cleanup on HTTP failure paths.

None is required to stop the current UI from lying. The next maintenance iteration should first restore the smallest honest contract: known totals get percentages; unknown totals remain unknown.

9 - A ListObjects Shortcut Must Not Turn a Missing Bucket into an Empty One

This document records the problem analysis, design discussion, and repair decision for SILO #32 and PR #37.

Status on 2026-08-26: PR #37 was updated to the DCO-signed head e9c5340be, formally approved, and merged as 49c8aeac4; #32 closed automatically. DCO, VulnCheck, and all six Go CI jobs passed on the exact PR head; the post-merge main VulnCheck and all six Go CI jobs also passed. No tagged release, package, container image, deployment, or production endpoint has yet been verified to contain the repair.
Scope: verify bucket existence only for three listing shortcuts that bypass storage; do not restore the generic checkBucketExist, change the normal listing path, or introduce an existence cache.
Release boundary: local commit, push, remote CI, merge, tag, package, container image, deployment, and production verification are independent gates.

Too Long; Didn’t Read (TL;DR)

The problem is real and worth fixing. A normal ListObjects, ListObjectsV2, or ListObjectVersions request against a missing bucket reaches storage and receives BucketNotFound. Three inputs, however, return early:

  • a marker outside the prefix;
  • max-keys=0;
  • a prefix beginning with /, including the Prefix="/" boto3 reproduction from #32.

Those branches return io.EOF directly. The caller treats EOF as a successful end of listing, so the client receives an empty 200 rather than S3’s 404 NoSuchBucket. The identity of the same missing resource changes from an error to success solely because the selection parameters differ. That breaks S3 compatibility and blocks a real user’s upgrade from the pre-regression release.

The repair must not put an expensive bucket check back in every listing. The selected design replaces only the three bare io.EOF returns with a small helper. The helper calls GetBucketInfo once: it returns the real error if the bucket is absent or cannot be confirmed, and preserves io.EOF when the bucket exists. The normal listing hot path is untouched. Only requests that would otherwise exit before storage pay the extra peer-and-disk fan-out.

That decision has now been executed: the strengthened repair passed local review, the exact PR head passed every remote check, and the expected-head-guarded merge entered a green main.

What is the problem?

One API exposes two bucket-existence semantics

#32 reproduces the defect by calling the following against a missing bucket:

s3.list_objects(Bucket="missing-bucket", Prefix="/")

AWS S3 raises NoSuchBucket; SILO returns a successful empty listing. The difference is not in authentication, routing, or XML serialization. It comes from the object-layer listPath control flow:

regular prefix
  -> enter listMerged
  -> consult storage
  -> missing volume/bucket becomes BucketNotFound
  -> HTTP 404 NoSuchBucket

shortcut input
  -> listPath returns io.EOF early
  -> storage is never consulted
  -> the caller treats EOF as normal completion
  -> HTTP 200 with an empty listing

/ is not the only trigger:

Shortcut condition Why the result must be empty Defect before the repair
Marker does not begin with the prefix The implementation does not scan this disjoint range Returns EOF without confirming the bucket
max-keys=0 The caller asks for zero keys Incorrectly equates “zero results” with “valid resource”
Prefix begins with / SILO’s flat key space produces no entries for this form The filter short-circuits before bucket identity

For an existing bucket, returning an empty listing from these branches is a reasonable optimization. For a missing bucket, the same EOF masks the resource error that should take precedence.

The regression has a known origin

The reporter confirmed correct behavior in RELEASE.2024-01-29T03-56-32Z and the regression beginning with RELEASE.2024-01-31T20-20-33Z. The corresponding upstream change is minio/minio#18917 / 80ca12008. It removed GetBucketInfo from generic argument checks and relied on actual Put, List, and Multipart storage operations to expose a missing bucket.

That optimization works on normal paths but leaves a gap: an early-return path never reaches the storage operation that is now responsible for producing the error. #32 does not require a broad rollback of the upstream optimization. It repairs the overlooked control-flow exits.

Why fix it?

The S3 contract explicitly requires NoSuchBucket

Both AWS ListObjects and ListObjectsV2 define NoSuchBucket as HTTP 404 when the specified bucket does not exist. prefix, marker, start-after, and max-keys select listing results; they must not turn a missing bucket identity into a successful request.

ListObjectVersions shares the same object-layer listing engine. Giving V1, V2, and version listings the same existence behavior on the same shortcut inputs prevents the three public APIs from diverging further.

An empty 200 changes client decisions

An empty 200 and a 404 are not interchangeable presentation details:

  • 404 tells provisioning or test code to create the bucket, fix configuration, or stop;
  • an empty 200 asserts that the bucket exists but has no matching objects;
  • SDKs, synchronization tools, and integration tests continue down different branches;
  • a test using SILO as an S3 substitute can pass locally and fail against AWS.

#32 also establishes a direct upgrade impact: an application relying on the older correct behavior cannot upgrade past the regression. The repair restores both S3 parity and upgrade compatibility.

The repair surface is narrow and testable

The bug is confined to three adjacent early returns. It does not involve object data, metadata formats, sorting, pagination-token encoding, permissions, or wire schemas. A very small production change can be pinned down with object-layer and HTTP-level contracts, so the benefit clearly exceeds the implementation risk.

Why not restore the global check?

Upstream did not remove generic GetBucketInfo as incidental cleanup. The motivation for #18917 states that checking the bucket before every Put, List, and Multipart operation fans out across servers; even after vectorization, the cost becomes visible beyond 100 nodes.

In current SILO, erasureServerPools.GetBucketInfo calls S3PeerSys.GetBucketInfo. That operation concurrently asks every peer and reduces quorum per pool, while each peer checks its local bucket state. It is not a cheap in-memory map lookup.

Two extremes are therefore unacceptable:

  • never check: keep the incorrect empty 200;
  • check before every List: restore semantics while undoing a critical large-cluster optimization.

The actual design question is whether the check can be confined to branches that never touch storage and therefore cannot discover the missing bucket naturally. It can.

How is it fixed?

Replace only three bare EOF returns

In cmd/metacache-server-pool.go, each shortcut previously executed:

return entries, io.EOF

It now executes:

return entries, z.listPathShortcutEOF(ctx, o.Bucket)

The helper has only two classes of outcome:

func (z *erasureServerPools) listPathShortcutEOF(ctx context.Context, bucket string) error {
    if _, err := z.GetBucketInfo(ctx, bucket, BucketOptions{}); err != nil {
        return err
    }
    return io.EOF
}
  • existing bucket: preserve the previous empty-list behavior;
  • missing bucket: pass BucketNotFound into the existing error mapping, producing HTTP 404 NoSuchBucket;
  • state cannot be confirmed: propagate quorum, offline, timeout, or context errors instead of fabricating success.

The normal listMerged, metacache scan, sorting, pagination, and response-generation paths do not change.

Why the helper belongs here

The check must sit next to the shortcut for three reasons:

  1. only this layer knows that it is about to bypass every storage access;
  2. moving it into generic argument validation charges every call;
  3. moving it into the scan layer cannot help because these branches never scan.

The name intentionally states the boundary. This is not a new generic checkBucketExist; it restores missing existence semantics immediately before a shortcut returns EOF.

Do not add a cache

A bucket-existence cache could reduce fan-out but immediately creates invalidation questions for create, delete, site replication, recovery, and expiry. Adding a second source of truth for three low-frequency shortcuts costs more complexity and consistency risk than it saves.

The selected implementation uses the existing GetBucketInfo source of truth. If future telemetry shows that large clusters receive frequent max-keys=0, slash-prefix, or disjoint-marker probes, the project can evaluate a dedicated metadata fast path, rate limiting, or a carefully invalidated cache using real data rather than speculative machinery in this compatibility patch.

Test and review evidence

Object-layer contract

The object-layer test runs against single-drive and multi-drive erasure setups and exercises four inputs:

  • slash-prefixed prefix;
  • zero limit;
  • marker outside prefix;
  • a regular prefix as a control that still receives the error naturally from storage.

Each case covers ListObjects, ListObjectsV2, and ListObjectVersions, using the typed isErrBucketNotFound predicate rather than brittle English error-string comparison.

HTTP contract

The handler test sends genuine signed requests for all three public APIs:

API Request shape Assertion
ListObjects GET /missing-bucket?prefix=/ HTTP 404 and XML code NoSuchBucket
ListObjectsV2 Add list-type=2 HTTP 404 and XML code NoSuchBucket
ListObjectVersions Add versions HTTP 404 and XML code NoSuchBucket

The HTTP test uses the real slash-prefix reproduction from #32. The other two shortcuts are enumerated at the object layer. This proves final wire behavior without repeating the full matrix in the slower handler fixture.

Local quality gates

The improved local commit passed:

go test ./cmd -count=1
the new object-layer and HTTP regressions (10 subcases)
focused go test -race
related existing listing tests
CGO_ENABLED=0 go build ./...
go vet ./...
CI-scope gofmt and git diff --check
post-commit focused regression rerun

The full local cmd test completed in 116.215 seconds. An independent local Claude Code review used the Fable model at Max effort to inspect the exact tree, call paths, error mapping, tests, performance boundary, and this decision. Its verdict was GO, with no mandatory pre-merge change.

The DCO-signed PR head e9c5340be then passed eight remote checks: DCO, VulnCheck, and six jobs in Go CI. After merge, the resulting main commit 49c8aeac4 independently passed VulnCheck and all six Go CI jobs. The slowest checks were PR cross-compile at 9 minutes 47 seconds and post-merge cross-compile at 9 minutes 30 seconds.

Can it introduce new problems?

Shortcut requests now fan out across the cluster

This is the most important and deliberately accepted cost. A shortcut on an existing bucket used to be little more than a local branch; it now calls GetBucketInfo. Directional local microbenchmarks observed:

Path Observed magnitude
Shortcut before the repair about 0.55 μs, 7 allocations
Repaired single-drive shortcut about 7.8–8.1 μs, 45–47 allocations
Repaired 32-drive shortcut about 70–81 μs, 977 allocations
Normal 32-drive listing about 0.95 ms

These numbers show local relative cost only; they are not a latency prediction for a 100+ node deployment. Real distributed execution adds peer networks, quorum, and slowest-node tail latency, potentially making the gap much larger. That is precisely why the check must not expand into the normal listing path.

The risk concentrates in malformed or probe-style traffic. A misconfigured client polling max-keys=0, a slash prefix, or disjoint markers at high frequency can amplify what was a cheap request into peer-and-disk work. After merge, the actual frequency of these inputs should be observed through S3 traces or metrics; rate limiting or optimization should follow evidence.

A degraded cluster exposes more real errors

Previously, a shortcut could return an empty 200 while peers were offline or bucket quorum was unavailable because it never consulted cluster state. The repair can return quorum, timeout, or service errors in those conditions.

That is more honest behavior, not an availability regression: if the server cannot establish that the bucket exists, it must not assert a valid empty bucket. Clients depending on unconditional empty success will nevertheless observe a behavior change.

Bucket create/delete races are not linearizable

GetBucketInfo and returning the empty result are two actions. The bucket can be deleted immediately after the check, or created immediately after a missing-bucket result is formed. This patch does not and should not add a transaction spanning bucket lifecycle to a listing shortcut.

This is the same concurrency class as other APIs that validate a resource before acting. The repair guarantees that the request no longer succeeds with no existence evidence at all; it does not promise a cross-node, cross-lifecycle linearizable snapshot of an empty listing.

Clients relying on the bug will receive 404

Some clients may have adopted the missing bucket’s empty 200 as fact. They will now enter an error branch. This is a visible compatibility change, but it restores the documented S3 contract and the pre-regression behavior. Preserving the bug merely transfers upgrade cost to clients that correctly rely on 404.

Two adjacent edges remain out of scope

The adversarial review recorded two non-blocking P3 boundaries:

  1. When resuming a metacache continuation, the c.fileNotFound branch still returns bare io.EOF. A stale or crafted continuation token used after bucket deletion could theoretically receive an empty 200. Adding GetBucketInfo there would affect normal continuation traffic and needs a separate performance and error-precedence design.
  2. Some V1 and version-list marker/prefix combinations return NotImplemented during HTTP handler validation before reaching the object layer; the V2 start-after route can reach it. This patch fixes storage shortcuts masking a missing bucket; it does not redefine precedence between malformed parameters and resource errors.

Neither blocks merge. The first is outside #32’s ordinary initial-list reproduction; the second is inherited handler behavior. Recording them prevents “all three shortcuts are covered” from being overstated as byte-for-byte AWS parity for every possible parameter combination.

Alternatives considered

Keep upstream behavior

This has zero performance change and minimizes fork divergence. It also keeps a documented S3 incompatibility, a regression with a known release boundary, and a misleading result when SILO is used as an integration-test substitute. For a narrow and well-tested compatibility repair, that tradeoff is no longer justified.

Restore generic checkBucketExist

This covers every path at once but reintroduces peer fan-out into every Put, List, and Multipart operation, directly undoing the large-cluster optimization from #18917. The cost is disproportionate and the option is rejected.

Fix only Prefix="/"

That passes the single issue reproduction but leaves the same root defect in max-keys=0 and marker-outside-prefix. The branches are adjacent and share the same semantics, so one helper is simpler and less likely to regress.

Add a bucket-existence cache

This makes shortcuts cheaper but requires semantics for create, delete, replication, recovery, and stale TTL windows. There is no telemetry showing enough shortcut traffic to justify that complexity, so it is not selected.

Complexity and cost-benefit

Dimension Assessment Rationale
Production-code complexity Low Three call sites and a seven-line helper; no new state, dependency, or format
Test complexity Low to medium V1, V2, versions, three shortcuts, a control, and HTTP mapping all need coverage
Normal-path risk Very low No check is added to the listMerged hot path
Shortcut runtime cost Materially higher A local EOF becomes cluster-wide GetBucketInfo
Compatibility value High Restores 404 NoSuchBucket, pre-regression behavior, and S3 test fidelity
Operational complexity Low No migration, configuration, feature flag, cache, or cross-repository dependency

The overall cost-benefit is favorable. The reason is not that GetBucketInfo is cheap—it is not—but that its cost is strictly limited to three shortcuts that otherwise cannot discover the missing bucket. A narrow performance cost in exchange for explicit protocol correctness is better than either a global rollback or indefinitely preserving the incorrect behavior.

Acceptance decision and remaining gates

The final decision was: accept and merge the strengthened PR #37 revision without expanding the production scope.

The accepted sequence was:

  1. replace the old fork head with the current-main, DCO-signed revision while preserving Jason Lin as a co-author;
  2. retain typed error predicates, V1/V2/version-list object-layer coverage, and HTTP-level 404 / NoSuchBucket assertions;
  3. update the PR description with the shortcut fan-out cost and unchanged normal-path boundary;
  4. approve the fork workflows and require all eight reported checks to pass on exact head e9c5340be;
  5. submit a formal approving review against that head;
  6. merge with an expected-head guard, producing 49c8aeac4, automatically close #32, and require the resulting main Go CI and VulnCheck to pass independently.

No cache, feature flag, additional abstraction, or continuation-token redesign was required. High-frequency shortcut traffic and large-cluster tail latency remain observability follow-ups, not reasons for speculative code expansion.

Repository integration is complete. A tag, package, docker.io/pgsty/minio image, deployment, and real S3-client verification must still complete before the repair can be described as delivered to users.

Conclusion

The issue is not merely “a slash prefix reports the wrong error.” The listing engine uses io.EOF to mean two different things: an empty result from an existing bucket and an early exit that never established whether the bucket exists. Removing generic existence checks for large-cluster performance was a sound upstream optimization, but the shortcuts violate its premise that a real storage operation will naturally surface a missing bucket.

The selected repair restores that premise by calling the existing GetBucketInfo only at three storage-bypassing exits. It makes those requests more expensive and exposes real errors on degraded clusters; both are explicit costs. In return, SILO restores S3’s 404 semantics, upgrade compatibility, and test fidelity while preserving the upstream optimization on the normal listing hot path.

This worthwhile, controlled compatibility fix is now merged and green on main; release delivery remains a separate gate.

10 - Optional Checksums, Mandatory Failure: Repairing UploadPart and UploadPartCopy Compatibility

This is the complete design and implementation record for SILO #46. The repair was not merely a changed if statement. One apparently optional S3 header reached into multipart completion semantics, copy responses, compression and encryption pipelines, compatibility baselines, and release verification.

Status: server implementation and local verification complete; commit, PR, remote CI, release, and production verification pending.
Owner: pgsty/silo, the SILO server repository.
Tracking: #46.
Independent follow-ups: #63 CopyObject + compression checksum, #64 federated UploadPartCopy checksum.
Adversarial review: local Claude Code, Fable 5, --effort max; final verdict GO, with no blocking findings.

Too Long; Didn’t Read (TL;DR)

A multipart upload splits a large file into smaller parts. A client may attach a checksum to each part so the server can verify the transfer, but AWS defines that checksum as optional. SILO used to treat it as mandatory: an ordinary UploadPart failed without one, and UploadPartCopy could never work because it has no part-body checksum to provide.

After the repair, SILO still validates a checksum when the client sends one. When the client omits it, SILO computes the checksum while reading the original bytes and saves the result. This happens before compression and encryption, requires no second read, and changes no on-disk format. The result is AWS-compatible behavior without weakening data integrity.

Decision

When a multipart upload declares a checksum algorithm in CreateMultipartUpload, SILO applies this contract:

  1. If the client supplies a part checksum, the server continues to validate it. A wrong value or algorithm fails and is never hidden by fallback computation.
  2. If the client omits the part checksum, the server computes it in one pass with the MPU algorithm over the logical plaintext stream, before compression and encryption, and persists the result.
  3. A normal UploadPart echoes a checksum response header only when the client supplied the checksum. A server-computed fallback is not echoed.
  4. UploadPartCopy has no client part-body checksum, so the server computes the value and returns it in CopyPartResult.
  5. ListParts returns the persisted part checksum.
  6. FULL_OBJECT completion continues to linearize the full checksum from stored part checksums. COMPOSITE completion continues to require a checksum for every part; clients can recover those values with ListParts.
  7. Computation occurs during the existing read. Completion never re-reads the entire object merely to manufacture missing state.

In one sentence:

The optional input is the client-provided checksum value, not the server’s responsibility to maintain a consistent checksum-enabled MPU.

How we found it

The defect surfaced while investigating a different multipart checksum issue, #31.

#31 concerned CompleteMultipartUpload: for FULL_OBJECT, a client can complete with part numbers, ETags, and an optional full-object checksum without retaining every part checksum in the completion XML. Tracing that path backward exposed a stronger, earlier condition in erasureObjects.PutObjectPart:

if cs := fi.Metadata[hash.MinIOMultipartChecksum]; cs != "" {
    if r.ContentCRCType().String() != cs {
        return InvalidArgument{/* checksum missing */}
    }
}

Once an MPU declared a checksum algorithm, every UploadPart had to carry the matching x-amz-checksum-* value. Omitting it returned:

400 InvalidArgument:
checksum missing, want "CRC32", got ""

API-level probes reproduced the behavior on both the single-drive and erasure backends.

Reviewing CopyObjectPartHandler raised the severity from a client-configuration incompatibility to P0. UploadPartCopy has no request body for the caller to checksum. The handler reads the source object, constructs an internal reader, and eventually enters the same PutObjectPart implementation. There is no client header and no SDK setting that can repair the request. Every checksum-enabled MPU therefore rejected UploadPartCopy by construction.

What AWS requires

This cannot be decided by saying that MinIO has historically behaved a certain way. The S3 protocol is the authority.

The AWS UploadPart API describes each algorithm-specific checksum header as something that “can be used as a data integrity check.” More importantly, its response fields say that the checksum is present only when it was provided in the request.

The AWS UploadPartCopy API is different: when the MPU was created with an algorithm, the copy result contains that part checksum. There is no copy request body, so this is necessarily a server-computed value.

The AWS ListParts API is the standard way to recover checksums for parts in an upload that is still in progress.

The algorithm/type matrix also rules out treating the repair as one Boolean flag:

Algorithm FULL_OBJECT COMPOSITE
CRC64NVME Supported Unsupported
CRC32 / CRC32C Supported Supported
SHA1 / SHA256 Unsupported Supported

FULL_OBJECT is limited to CRCs that can be linearized, but SHA1 and SHA256 still need correct per-part digests for COMPOSITE completion.

SDK configuration makes the gap practical. Current AWS SDKs usually calculate request checksums when an operation supports them, but users can choose request_checksum_calculation = when_required, and low-level callers can initiate an algorithm without repeating it on every part. S3 accepts those requests; SILO did not.

Why removing the check is not a fix

The most tempting patch is to delete the comparison and allow a checksum-less part to proceed. That only moves the failure to completion.

SILO does not reconstruct and re-read all object bytes during MPU completion. It reads ObjectPartInfo.Checksums from each part.N.meta:

  • a missing entry immediately becomes InvalidPart;
  • FULL_OBJECT calls Checksum.AddPart, combining digests with their part lengths;
  • COMPOSITE concatenates the raw digest bytes and hashes them into the object checksum.

The actual invariant is therefore:

checksum-enabled MPU
        => every committed part has a checksum for the MPU algorithm

Deleting the upload check without filling the metadata would make UploadPart appear successful, leave ListParts incomplete, omit the UploadPartCopy response value, and fail later during completion. A delayed failure is harder to diagnose than the original immediate one.

Alternatives considered

Option Benefit Fatal problem Decision
Delete the strict check Smallest diff Part metadata still lacks the checksum; completion must fail Rejected
Relax only FULL_OBJECT Unblocks some default CRC clients Leaves COMPOSITE and SHA incompatible; cannot close #46 Rejected
Re-read every part at completion Avoids storing a digest during upload Adds O(object size) second-pass I/O and still cannot fix ListParts or the copy response Rejected
Always return the server value from normal UploadPart Makes federation forwarding easy Violates the AWS response contract Rejected
Copy the AIStor implementation exactly Commercial precedent CRC-only fallback and a transformed-stream placement risk Rejected
Compute and persist in one pass over logical plaintext Complete protocol behavior, no second I/O, CRC and SHA support Requires an explicit plaintext checksum reader distinct from the storage reader Accepted

What the commercial edition taught us

We downloaded and verified the then-current MinIO AIStor RELEASE.2026-08-07T18-34-35Z. Without a commercial license the server enters offline mode and denies S3 operations, so the evidence came from Go pclntab and ARM64 disassembly, not a black-box compatibility run.

The static analysis showed that AIStor already:

  • installs a server hasher when the client checksum is absent;
  • persists the result in part metadata;
  • exposes checksum fields in CopyPartResult.

It nevertheless applies fallback only to CanMerge() algorithms—CRC32, CRC32C, and CRC64NVME. SHA1/SHA256 COMPOSITE still follows the old checksum missing path. More importantly, the hasher is attached in the object layer to the current r.Reader; under compression or encryption that reader may already represent transformed storage bytes.

AIStor validated the general direction—compute and store—but not an implementation that SILO could copy mechanically.

How adversarial review overturned the first design

The first plan tried to centralize every decision inside erasureObjects.PutObjectPart: read the MPU metadata in the object layer and install a server hasher when the incoming reader had no client checksum. It looked attractive because all internal callers would share one rule.

The first Fable 5 Max adversarial review found that this design was wrong for compression.

newS2CompressReader is not a lazy wrapper. Construction immediately launches a goroutine:

go func() {
    _, err := io.Copy(comp, r)
    // ...
}()

The S2 writer also reads several blocks concurrently. After constructing the compressor, the handler still performs option parsing, encryption preparation, and the object-layer call. By the time PutObjectPart installed a hasher, the plaintext reader could already have lost several MiB:

  • a large part would get a checksum with a missing prefix;
  • a small part could reach EOF before installation and produce no result;
  • mutating ServerSideHasher concurrently with Read would be a data race.

That finding changed the responsibility split:

The handler installs the hasher before any eager transform starts; the object layer validates the algorithm, requires a result, and persists it atomically.

This was the decisive turn in the design. Putting logic in the lowest layer may look more uniform, but stream correctness depends equally on when bytes begin moving and which representation of those bytes a layer can see.

Final implementation

A dedicated logical checksum reader

PutObjReader originally distinguished two concepts:

  • Reader, the stream sent to storage, possibly compressed or encrypted;
  • rawReader, used by older ETag and checksum code.

Under compression, even rawReader may not directly see plaintext; it can merely carry an ETag through an etag.Tagger chain. The repair therefore did not overload it. It added an unexported field:

checksumReader *hash.Reader

This reader always represents the logical S3 part bytes. WithEncryption can replace the storage Reader, but it must preserve checksumReader.

Unexported accessors on PutObjReader then:

  • return the effective client or server checksum type;
  • prefer the client value whenever it exists;
  • otherwise return the server result finalized at EOF.

Keeping the mechanism unexported minimizes public Go API growth and gives #63 a shared internal path without prematurely changing ordinary CopyObject behavior.

Preparing the hasher before transformations

prepareMultipartChecksumReader loads the algorithm and checksum type saved with the MPU:

  1. no declared algorithm means no work;
  2. an existing client checksum is compared by base algorithm;
  3. a wrong algorithm preserves the InvalidArgument rejection;
  4. an omitted client checksum installs the corresponding server hasher on the plaintext reader.

For normal UploadPart:

  • the compressed path prepares actualReader after request-checksum parsing but before newS2CompressReader;
  • the uncompressed path prepares the request hash reader before the encryption reader is constructed.

For UploadPartCopy:

  • a checksum-enabled MPU first gets an inner hash reader over the logical source range;
  • a range copy hashes only the selected bytes;
  • compression and destination encryption start only after that reader is ready.

The object layer remains authoritative

Early handler preparation does not replace the storage invariant. erasureObjects.PutObjectPart still:

  • re-parses the expected MPU algorithm;
  • requires an effective checksum type that matches;
  • obtains the checksum map after erasure encoding finishes;
  • reports an internal error instead of committing if an enabled algorithm has no result;
  • writes the checksum with the ETag, sizes, and index into part.N.meta, then atomically renames the part.

An internal caller that bypasses the HTTP handler without preparing a valid checksum is therefore rejected just as before. It cannot silently commit a part that violates the MPU invariant.

CopyPart response shape

CopyObjectPartResponse gained the five algorithms supported by this source tree:

ChecksumCRC32
ChecksumCRC32C
ChecksumCRC64NVME
ChecksumSHA1
ChecksumSHA256

All are omitempty, so an MPU without checksums produces the old XML. Normal UploadPart still uses the existing TransferChecksumHeader and echoes only a client request value; fallback computation does not alter that response.

Why it works

After the repair, the data flow is:

logical plaintext part
        |
        +--> client checksum verifier (if supplied)
        |         or
        +--> server-side hasher (if omitted)
        |
        v
compression (optional)
        |
        v
encryption (optional)
        |
        v
erasure encode / storage
        |
        v
persist ETag + size + logical part checksum atomically

This satisfies four requirements that previously appeared to conflict:

  1. Protocol compatibility: omitting an optional header succeeds.
  2. No integrity downgrade: a supplied client value is still checked end to end and is never hidden by server fallback.
  3. Correct object semantics: the checksum covers logical S3 bytes, not compressed data or ciphertext.
  4. Controlled cost: hashing shares the existing read and adds CPU, not a second disk or network pass.

EOF has a precise role. hash.Reader finalizes ServerSideChecksumResult only when it reaches EOF. Closing the compression pipe synchronizes the compressor goroutine with the storage read; the object layer reads the result only after encoding returns. Targeted -race tests verified that concurrency boundary.

The compatibility-baseline blocker

The five new CopyObjectPartResponse fields are exported Go API. SILO’s buildscripts/rebrand-guard rescans imports, environment variables, headers, routes, storage markers, and exported symbols, then compares them in both directions with buildscripts/rebrand-guard/compat-baseline.json. An unacknowledged symbol makes CI fail.

After recording the five #46 fields, the guard still reported two additions:

internal/config/notify:notify:type:LegacyDatabaseTargetError
internal/config/notify:notify:method:LegacyDatabaseTargetError.Error

They did not come from #46. They belong to the earlier database-notification repair f1ba68358 on the local main branch. The cmd startup path intentionally needs the exported type for errors.As, but that earlier commit had not updated the compatibility baseline. Every later change based on that HEAD would therefore fail the CI guard.

We chose “option A”: acknowledge the two notification symbols as part of their original repair while retaining the five #46 fields. The final baseline diff is exactly seven additions and zero deletions, and the guard reports:

exported=9021
Silo rebrand compatibility baseline is unchanged

This does not disable the check. Exact set equality means that acknowledging a nonexistent symbol also fails. The change explicitly records two intentional compatibility-surface additions.

golangci-lint has not yet run locally; it remains a remote go.yml gate. Green local go test, go vet, race, and rebrand-guard results do not substitute for green remote CI.

Verification evidence

The new tests execute 76 subtests across:

  • CRC32, CRC32C, and CRC64NVME FULL_OBJECT;
  • CRC32, SHA1, and SHA256 COMPOSITE;
  • correct client checksums, wrong algorithms, and wrong values;
  • absence of a server-computed checksum in normal UploadPart responses;
  • server values in UploadPartCopy responses and ListParts;
  • a real 5 MiB + 1 KiB two-part full-object merge;
  • zero-length parts and overwriting the same part number;
  • a range copy whose SHA256 covers only the copied interval;
  • single-drive and 16-drive erasure backends;
  • default, versioned, compressed, encrypted, and compressed-plus-encrypted modes;
  • explicit SSE-C and SSE-S3.

Local validation included:

go test -race ./cmd -run '^TestAPIUploadPartServerSideChecksum' -count=1
go test ./cmd -count=1
go test ./... -count=1
go vet ./cmd
git diff --check
go run ./buildscripts/rebrand-guard

All passed. Two subsequent Claude Code Fable 5 Max implementation reviews and the final acceptance review returned GO with no blocking findings.

Cost, risk, and release boundary

When a client omits its value, the server performs one additional hash over the part. CRC cost is small; SHA costs more CPU. Both share the read that already had to occur, without buffering an entire part in memory or adding a completion-time second pass.

During a rolling upgrade, old and new nodes may answer the same checksum-less request differently: a new node accepts it while an old node returns 400. ObjectPartInfo.Checksums did not change format, so stored data remains downgrade-readable, but client-visible behavior stabilizes only after all serving nodes have upgraded. The release note must call that out.

This record describes a local main worktree. The implementation has not been committed, pushed, run through remote CI, or packaged into a release. SILO documentation belongs to silo.pgsty.com; a successful local Hugo build does not mean that the product in the wider pgsty.com ecosystem has shipped.

Why two follow-ups remain separate

Adversarial review found two related but independent issues.

#63: CopyObject + compression

Ordinary CopyObject can also attach a server-side checksum to a transformed stream. It shares the root cause and the new checksumReader mechanism, but it is a different API with a different test matrix and rollback boundary. We chose a separate repair and require that PR to reuse this plaintext-reader contract instead of inventing a second abstraction.

#64: legacy federation

Legacy etcd federation turns UploadPartCopy into an ordinary remote UploadPart. Under the AWS response semantics preserved here, that remote request does not return a server fallback value, so the proxy may still lack the checksum required for CopyPartResult. A follow-up must independently choose between a remote-returned value and an ETag-verified ListParts fallback. It must not make all external UploadPart responses non-compliant merely to simplify an internal proxy.

Separating them does not abandon consistency. Consistency is maintained through one shared rule:

Every server-computed S3 checksum binds to the logical plaintext stream, is installed before any eager transform, and is validated and persisted by the object layer that owns the storage invariant.

Lessons retained

The repair leaves lessons more durable than its individual lines of code:

  1. An optional header does not make internal state optional. If the protocol lets the client omit a value, the server must produce the state its own completion path needs.
  2. Request acceptance and response disclosure are separate contracts. A normal UploadPart may compute internally and still omit the value; UploadPartCopy must return it.
  3. Stream layers are defined by byte semantics. The lowest layer is not automatically correct if it no longer sees logical bytes, and an eager goroutine turns “install later” into a race.
  4. A commercial implementation is evidence, not the specification. AIStor showed the direction and the boundary that could not be copied.
  5. A compatibility guard is a change-acknowledgment mechanism. compat-baseline.json exists to assign every new compatibility surface, not merely to make CI quiet.
  6. Independent defects should ship independently while sharing invariants. #63 and #64 remain separate, but both must cite and obey the checksum-reader contract established here.

The final result is not a broad relaxation. It is a stricter and more accurate boundary: clients may omit optional information; the server may not omit correctness.

11 - BadDigest, InvalidRequest, and the CompleteMultipartUpload Checksum Contract

This is the design, investigation, and verification record for SILO #48, with the decision boundary for the related SILO #50.

Status: pgsty/silo#74 merged as 590aeaa7d, and pgsty/silo.pgsty.com#6 merged as 9805dd7; full local verification, remote CI, and independent Opus 5 Max acceptance review completed on the linked changes. Tag, release, package, image, deployment, and production verification remain separate pending gates.
2026-08-28 follow-up: signed-off server commit f7bc725d8 closes the remaining type-only and invalid-token bypass without changing CRC64NVME canonicalization. Complete local, tagged, race, static, build, and Fable Max verification passed; push, remote CI, merge, tag, and delivery remain pending.
Owner: pgsty/silo, the SILO server repository.
Implementation scope: CompleteMultipartUpload error semantics only; no storage-format, checksum-math, dependency, Console, package, or client change.
Independent decision: #50 remains probe-gated and is not part of this repair.

Too Long; Didn’t Read (TL;DR)

Issue #48 is valid and should be fixed, with two corrections to the original report.

First, the checksum-type comparison is worse than the issue states. SILO used bitmask containment instead of equality. An upload created as FULL_OBJECT and completed as COMPOSITE failed, but the reverse COMPOSITE to FULL_OBJECT direction could pass the type check. The repair must compare the base algorithm and normalized multipart object type independently and symmetrically.

Second, the missing-part-checksum row originally lacked a direct AWS capture. That evidence now exists in the official boto/s3transfer project: issue #241 records a real S3 InvalidRequest response naming sha256 and missing part 1, and PR #242 repaired the client and added tests. This is strong enough to implement the response contract without a new AWS account probe.

The accepted behavior is:

CompleteMultipartUpload failure SILO before Required behavior
Supplied object checksum does not match the assembled object XAmzContentChecksumMismatch BadDigest
Completion checksum type differs from initiation, in either direction one direction InvalidArgument; reverse direction could pass BadDigest
Completion declares a different type but sends no whole-object checksum type assertion ignored BadDigest
Completion sends an unknown non-empty type, with or without a checksum value could be ignored or interpreted through checksum defaults InvalidArgument
A composite completion omits a checksum for a part InvalidPart InvalidRequest, naming the algorithm and part

The repair uses completion-specific error types. It deliberately does not change the global mapping of hash.ChecksumMismatch, so PutObject, UploadPart, streaming trailers, and other operations retain their existing XAmzContentChecksumMismatch contract.

Issue #50 is a separate question. AWS documents that CRC64NVME is full-object only, but the available sources do not prove that S3 rejects an explicit CRC64NVME + COMPOSITE initiation instead of canonicalizing it. Upstream MinIO intentionally implemented canonicalization and exposes FULL_OBJECT in the initiation response, so the behavior is not silent. A raw AWS probe is required before changing it.

Scope and decision

This record answers two different questions:

  1. Are the #48 error-code deviations real, externally observable compatibility defects with enough evidence to repair?
  2. Does the same evidence authorize changing the CRC64NVME canonicalization described by #50?

The decisions are:

  • #48: accept with corrections and implement. The error codes are part of the S3 wire contract. Returning a different code makes SDK behavior and operator diagnosis diverge even when the request is rejected in both systems.
  • #50: do not implement yet. The capability matrix proves the resulting checksum must be full-object. It does not establish whether an invalid requested type is rejected, ignored, or canonicalized. Those are different wire contracts.

The repair is intentionally narrow. It does not add algorithms, recalculate stored data, reinterpret successful uploads, or change the optionality rules repaired for #31 and #46.

Evidence ledger

Not all evidence has the same authority. The implementation decision uses the following hierarchy.

Grade Source What it establishes Limitation
A AWS checksum upload guide A supplied full-object checksum mismatch fails with BadDigest; algorithm/type capability matrix Does not show every response message
A AWS CompleteMultipartUpload API and AWS CLI reference A completion checksum type that differs from initiation fails with BadDigest Does not publish the exact message text
B+ boto/s3transfer #241 Real AWS S3 transcript: missing SHA256 checksum for part 1 returns InvalidRequest and names the algorithm and part Captured in an official SDK project issue rather than an AWS API reference page
B+ boto/s3transfer #242 and the 0.6.1 changelog The official transfer client was changed to forward UploadPartCopy checksums into completion; functional coverage prevents recurrence Primarily client-side evidence
B Local API probes and regression tests SILO’s old XAmzContentChecksumMismatch, InvalidArgument, InvalidPart, and reverse-direction bypass are reproducible on both object-layer backends Establishes SILO, not AWS
C Upstream MinIO history Explains how the current behavior entered the lineage and why it remains Intent is not proof of AWS parity

This distinction matters. The original #48 comment correctly downgraded the third row while it was supported only by secondary reports. The boto transcript and the merged client repair close that evidence gap.

The observable contract

Object checksum mismatch

For a FULL_OBJECT multipart upload, SILO combines stored part checksums and compares the result with the optional object checksum supplied on completion. The old code returned hash.ChecksumMismatch. A global API mapping converted that type to:

400 XAmzContentChecksumMismatch

AWS explicitly documents BadDigest for the corresponding completion integrity failure. Reusing the existing generic ErrBadDigest code without a custom message would still be misleading because its static text says Content-MD5; CRC32, CRC32C, and CRC64NVME are not Content-MD5.

The new response is therefore operation-specific:

400 BadDigest
The CRC32 checksum you specified did not match the calculated checksum.

The response does not disclose the expected or supplied digest.

Checksum type mismatch

The checksum type saved by CreateMultipartUpload is part of the upload’s contract. A completion may not switch between COMPOSITE and FULL_OBJECT.

The old test was:

!provided.Type.Is(expectedType)

ChecksumType.Is is a containment operation over a bitmask, not equality. For CRC32:

created FULL_OBJECT + completed COMPOSITE => rejected
created COMPOSITE   + completed FULL_OBJECT => containment succeeds

The second request could proceed using the persisted composite rules. If the caller supplied the composite checksum value under a FULL_OBJECT declaration, completion could even succeed. This is a protocol validation bypass, not merely the wrong error label.

The repair normalizes both values into multipart checksum types, then compares:

  1. base algorithm equality; and
  2. object type equality (COMPOSITE versus FULL_OBJECT).

For algorithms whose two object-type forms are both syntactically accepted—currently CRC32 and CRC32C—both mismatch directions now return 400 BadDigest. SHA1 and SHA256 with FULL_OBJECT are rejected earlier as InvalidArgument; CRC64NVME is the canonicalized special case discussed under #50 below. Base-algorithm mismatch remains a separate InvalidArgument path because #48 and the cited AWS type contract do not authorize broadening that behavior.

Type-only assertions and invalid tokens

The first #48 repair remembered whether x-amz-checksum-type was present, but its object-layer comparison was still nested under WantChecksum != nil. WantChecksum is populated only when completion carries a checksum value. A caller could therefore send a type assertion without a whole-object checksum:

CreateMultipartUpload:   CRC32 + COMPOSITE
CompleteMultipartUpload: x-amz-checksum-type: FULL_OBJECT
                         no x-amz-checksum-crc32 value

The server returned success and persisted the initiated composite state. It did not corrupt the object, but it accepted an explicit integrity assertion that contradicted the upload contract.

There was a second parser asymmetry. In the header-without-algorithm path used by completion, an unknown value such as NOT_A_TYPE could be ignored when a checksum header was also present. Relying on ChecksumType.ObjType() after creating an invalid bitmask would not be safe: an invalid non-multipart value can fall through to the full-object default. Raw enum validation must happen first.

The follow-up stores the explicit raw type string in ObjectOptions, accepts only COMPOSITE or FULL_OBJECT, and compares it with the initiated multipart type independently of WantChecksum. The order is deliberate:

  1. reject every unknown non-empty token as InvalidArgument;
  2. compare the base algorithm when a checksum value is supplied;
  3. compare the explicit object type whenever the upload recorded a checksum algorithm;
  4. report an explicit type mismatch as BadDigest even when no object checksum value was supplied.

CRC64NVME remains a deliberate exception. A raw COMPOSITE token is normalized to FULL_OBJECT before comparison, preserving the inherited behavior pending the #50 AWS probe. A legal type-only header on an upload that recorded no checksum algorithm remains outside the comparison because there is no initiated checksum type to assert against; its exact AWS error semantics remain unproven and were not expanded into this repair.

Missing composite part checksum

For a composite upload, the completion XML must include the selected checksum for every listed part. SILO previously compared an empty client value with the stored part checksum and returned InvalidPart.

That conflated three different states:

  • the part or ETag does not exist;
  • a checksum was supplied but has the wrong value or algorithm;
  • the required checksum element is absent.

The third state now has a dedicated error. Its wire message follows the AWS response captured by boto/s3transfer:

400 InvalidRequest
The upload was created using a sha256 checksum. The complete request must include
the checksum for each part. It was missing for part 1 in the request.

The error is emitted for the first missing part and includes its actual number. FULL_OBJECT behavior is unchanged: a completion may omit per-part checksum elements, while any supplied part checksum must still be valid.

Root cause in the upstream lineage

The behavior is inherited rather than a SILO-specific redesign.

  • MinIO PR #15433 introduced extended checksum handling and the global hash.ChecksumMismatch to XAmzContentChecksumMismatch mapping. That mapping is suitable for streaming upload validation but too broad for completion semantics.
  • MinIO PR #20855 added full-object checksums and CRC64NVME. It introduced the checksum-type comparison and intentionally canonicalized CRC64NVME to full-object with the comment that AWS appears to ignore the supplied mode.
  • MinIO PR #20953 tightened invalid algorithm/type combinations but retained the CRC64NVME special case. That is evidence of deliberate upstream behavior, not an accidental missing branch.
  • MinIO issue #20944 reported an AWS BadDigest versus MinIO InvalidPart difference. The divergence was acknowledged but not repaired.

The upstream repository is now archived. SILO therefore owns the compatibility decision, tests, and maintenance burden rather than waiting for an upstream correction.

Repair design

Operation-scoped errors

Changing the global hash.ChecksumMismatch mapping would alter every operation that uses it. That would be a larger, weakly evidenced compatibility change.

The repair adds three package-private, sentinel-backed error helpers in the server command package. Keeping the helpers and the request-header-presence flag private avoids expanding SILO’s exported Go compatibility surface:

  • completeMultipartChecksumMismatch, mapped to BadDigest with a checksum-aware description;
  • completeMultipartChecksumTypeMismatch, mapped to BadDigest with provided and initiated types;
  • missingPartChecksum, mapped to InvalidRequest with algorithm and part number.

Only CompleteMultipartUpload produces these types. The global mapping remains:

hash.ChecksumMismatch => XAmzContentChecksumMismatch

This preserves PutObject and UploadPart behavior and makes the compatibility boundary visible in code.

Symmetric type validation

Both persisted and supplied types are normalized with the multipart flags before comparison. This is necessary because a bare CRC checksum type describes a non-multipart full-object checksum through ObjType(), while the same base value means composite after multipart context is applied. Object type is compared only when the completion request explicitly contains x-amz-checksum-type; omitting an optional header does not synthesize a COMPOSITE assertion.

The resulting invariant is:

provided base algorithm == initiated base algorithm
AND
provided multipart object type == initiated multipart object type

The second condition applies only to an explicitly supplied type. This comparison is symmetric and remains compatible with the existing CRC64NVME canonicalization. It fixes #48 without silently deciding #50.

Precise missing-value detection

For each part, the server already builds a map of all checksum fields supplied in the completion XML. The repair distinguishes:

COMPOSITE + no checksum field supplied => missingPartChecksum / InvalidRequest
expected field present but wrong value => InvalidPart
only a different algorithm supplied    => InvalidPart
FULL_OBJECT + no checksum field         => allowed
FULL_OBJECT + any checksum field        => validate it

This is intentionally narrower than converting every part checksum failure to InvalidRequest. Only the state demonstrated by AWS evidence changes.

The same edit corrects the internal InvalidPart expected/actual field order. The generic S3 InvalidPart wire response did not expose those digest values, but internal error text and logs should still describe them correctly.

Regression and detection matrix

The API-level tests exercise signed HTTP requests through both the single-drive and erasure object-layer backends.

Test Request Required assertion
Full-object digest mismatch Correct parts, wrong object CRC32 HTTP 400, BadDigest, checksum-aware message, no object committed
Composite object digest mismatch Correct CRC32 part values, wrong composite object value HTTP 400, BadDigest; covers the separate checksum-of-checksums path
Type mismatch: full to composite Initiate CRC32 FULL_OBJECT, complete COMPOSITE HTTP 400, BadDigest, provided/expected types named
Type mismatch: composite to full Initiate CRC32 COMPOSITE, complete FULL_OBJECT HTTP 400, BadDigest; closes old containment bypass
Type-only mismatch in both directions Initiate one CRC32 type; complete with the opposite type and no object checksum value HTTP 400, BadDigest; the explicit assertion cannot bypass validation by omitting the digest
Invalid explicit type Complete with NOT_A_TYPE or lowercase full_object, with and without a checksum value HTTP 400, InvalidArgument, no object committed
Matching type-only assertion Initiate and complete CRC32 COMPOSITE, omit object checksum value Success; the valid assertion is enforced without inventing a required digest
Omitted optional type Initiate FULL_OBJECT, complete with checksum value but no type header Success; omission is not treated as explicit COMPOSITE
Algorithm mismatch guard Initiate CRC32, complete with CRC32C Still InvalidArgument
CRC64NVME #50 guard Initiate CRC64NVME with explicit COMPOSITE, then complete with explicit COMPOSITE Still succeeds through existing full-object canonicalization; records the completion-side residue rather than claiming #48 validates the raw type token
Missing composite checksum CRC32 and SHA256 composite uploads; omit all values, then omit only part 2 HTTP 400, InvalidRequest, lowercase algorithm and actual missing part named
Global-mapping guard Direct hash.ChecksumMismatch mapping Still XAmzContentChecksumMismatch
UploadPart guard Wrong client part checksum Still XAmzContentChecksumMismatch

The committed type-mismatch regression uses CRC32, while an independent acceptance probe covered CRC32C as well. The follow-up additionally covers type-only, unknown, lowercase, matching, and checksum-bearing invalid-token cases. The same matrix confirms that SHA1/SHA256 FULL_OBJECT requests stop earlier at the existing invalid-combination check and that CRC64NVME still canonicalizes an explicit COMPOSITE token. Those distinctions are protocol boundaries, not untested claims that every algorithm reaches the same error mapper.

Focused verification command:

go test ./cmd -run 'TestAPIErrCode$|TestAPICompleteMultipart(FullObjectChecksumMismatch|CompositeStillRequiresPartChecksums|CompositeChecksumMismatch|ChecksumTypeMismatch)$|TestAPIUploadPartServerSideChecksumDoesNotMaskClientErrors$' -count=1

Observed result on 2026-08-27:

ok  github.com/minio/minio/cmd

The complete local package gate was then rerun after the review-driven additions:

go test ./cmd ./internal/hash -count=1
ok  github.com/minio/minio/cmd           121.462s
ok  github.com/minio/minio/internal/hash   0.566s

git diff --check also passed. Independent review of the final diff remains a separate gate. A local pass is not remote CI, a merged commit is not a release, and a release is not production deployment.

Independent adversarial review

The first review of the actual server diff was performed with local Claude Code in read-only safe mode. Its verdict was GO with no blocking findings. It independently confirmed the operation-scoped mapping, symmetric bitmask normalization, per-part missing-value detection, both object-layer backends, and preservation of UploadPart behavior.

The review identified four useful gaps that were incorporated before the second full test run:

  • distinguish an omitted optional type header from an explicit COMPOSITE assertion;
  • separate value-mismatch and type-mismatch error types;
  • exercise the composite checksum-of-checksums mismatch path;
  • pin missing part 2, algorithm mismatch, and unchanged CRC64NVME canonicalization.

One first-review concern was rejected by primary evidence: it questioned whether checksum type mismatch should return InvalidRequest. The AWS CompleteMultipartUpload reference and AWS CLI reference explicitly specify BadDigest when the completion type differs from initiation.

The final first-round re-review verdict was FINAL GO, no blockers. It explicitly withdrew the earlier error-code concern, agreed with accepting #48 and deferring #50, verified that the new guards preserve the intended non-changes, and found no English/Chinese drift.

A subsequent independent acceptance run used Claude Code claude-opus-5 with maximum effort. It returned ACCEPT, no blocking findings, reproduced the old composite-as-FULL_OBJECT bypass end to end against the pre-fix code, verified that the new API assertions fail against that code, and probed all five checksum algorithms in both type directions.

The 2026-08-28 follow-up received a separate local Fable Max mirror review over the complete uncommitted release-review diff. It returned GO, with no P0–P2 findings. The primary review independently checked its seven P3 observations: five were non-blocking boundaries, while two proposed causes were disproved by the actual config and key-rotation call paths. The review confirmed that raw invalid types are rejected before normalization, type-only mismatch is enforced, source-side checksum decryption still receives the full request, and CRC64NVME canonicalization remains untouched.

Five pre-existing or deliberately deferred, non-blocking observations remain outside these repairs:

  • SHA1/SHA256 FULL_OBJECT combinations are rejected by the existing parser as InvalidArgument before the new type-mismatch mapper; only CRC32/CRC32C reach both mismatch directions;
  • CRC64NVME treats any type value as full-object state, so completion with an explicit COMPOSITE token is still accepted through canonicalization pending the #50 AWS probe;
  • when initiation recorded no checksum algorithm but completion supplies an object checksum, SILO returns BadDigest; AWS documentation says such a value is accepted and ignored, so this should be triaged as a separate compatibility issue;
  • composite part-count and value mismatches both become BadDigest with the same description;
  • a full-object checksum carrying a -N suffix has that suffix ignored while its digest is still validated.

None is introduced by these patches, and none changes the #48 decision. They should be triaged separately if strict message or invalid-header parity becomes a maintenance priority.

Why #50 is not included

Issue #50 says CRC64NVME + COMPOSITE should be rejected at initiation. Three facts are confirmed:

  1. AWS’s algorithm matrix supports CRC64NVME only as a full-object checksum.
  2. SILO and upstream MinIO canonicalize the request to full-object state.
  3. The server returns x-amz-checksum-type: FULL_OBJECT from CreateMultipartUpload, so the substitution is externally visible rather than silent.

What is not confirmed is the decisive wire behavior: does AWS reject the explicit invalid combination, or accept it and return/carry full-object state? A capability matrix does not answer that question.

The upstream history also argues against guessing. PR #20855 added the canonicalization intentionally, and PR #20953 preserved it while tightening other invalid combinations. That may be based on an AWS observation, but the comment is not a reproducible transcript.

The same representation also affects completion: FullObjectRequested treats every CRC64NVME checksum as full-object state, so a stored FULL_OBJECT upload completed with the raw header value COMPOSITE is accepted as full-object rather than rejected as a type mismatch. This completion-side residue falls under the same raw-token-versus-canonical-state evidence question. It is explicitly not claimed fixed by #48.

PutObject must not be bundled into this decision. Its API reference does not define x-amz-checksum-type, so accepting, rejecting, or ignoring that header is a separate undocumented-header question.

Required AWS probe

Before changing #50, capture a raw SigV4 request and response against a general-purpose AWS S3 bucket:

  1. send CreateMultipartUpload with x-amz-checksum-algorithm: CRC64NVME and x-amz-checksum-type: COMPOSITE;
  2. record the HTTP status, error code/message, request ID, and all checksum response headers;
  3. if accepted, upload one part and complete it, recording whether S3 requires per-part values and which type HeadObject reports;
  4. repeat with FULL_OBJECT as the control;
  5. probe PutObject separately, explicitly labeling it as an undocumented-header experiment.

Only a captured rejection authorizes replacing canonicalization with validation. If AWS accepts and canonicalizes, #50 should be corrected or closed rather than implemented.

Compatibility and operational impact

  • Successful requests: checksum semantics are unchanged, except that omitting the optional x-amz-checksum-type header is no longer misclassified as an explicit COMPOSITE assertion. That intentional interoperability relaxation changes the old erroneous 400 into success.
  • Rejected requests: apart from that omitted-header case, HTTP status remains 400; the affected S3 error code and message become AWS-compatible. Explicit type-only mismatch is now enforced, and an unknown non-empty type is rejected as InvalidArgument before bitmask normalization.
  • Integrity: unchanged or stronger. The reverse type-bypass is closed; no failed completion commits an object.
  • Stored data: no format, checksum encoding, metadata, erasure layout, migration, or backfill change.
  • Performance: constant-time comparisons and error construction only; no additional data reads or hashing passes.
  • Security/privacy: digest values are not returned in the new messages. Bucket and object names are not added to them.
  • Rolling upgrade: nodes may return different error codes until all serving nodes are upgraded, but successful objects remain compatible.
  • Rollback: restores the old error codes and asymmetric check; it does not require data rollback.
  • Other repositories: no Console, shared-package, MCLI, or SDK change is required. This public design record is the only cross-repository deliverable.

Merge and release gates

Gate Base #48 repair 2026-08-28 follow-up
Design and local verification complete complete
Independent adversarial review complete, ACCEPT complete, GO
Signed-off server commit complete local f7bc725d8
Push, remote CI, and merge merged as 590aeaa7d not established
Public design record merged as 9805dd7 this documentation update is local
Tag and release artifacts not established not established
Container image and package not established not established
Deployment and production probe not established not established

The follow-up must keep #50 out unless a raw AWS transcript changes the decision, run remote DCO/Go CI/vulnerability/release-pipeline checks on its final commit, and merge from the current SILO main. Repository integration, release artifact, image, deployment, and production probe remain independent gates; none can be inferred from a local test or documentation build.

Conclusion

#48 is a correct compatibility issue, and the evidence now covers all three rows. The safest repair does not relabel checksum failures globally. It teaches CompleteMultipartUpload to report its own protocol errors, compares checksum types symmetrically, and identifies a genuinely missing composite part checksum without confusing it with a missing part or a wrong value.

#50 is related by discovery history, not by proof. The server’s current CRC64NVME canonicalization is deliberate and visible. Until AWS’s exact response is captured, changing it would replace one unverified assumption with another.

That boundary is the central design decision: implement what the official contract and tests establish, test the hidden consequence found in the code, and leave the remaining policy question behind an explicit, reproducible evidence gate.

12 - Per-Bucket CORS: Making Deletes and Recovery Converge

This document records the problem, review, merge decision, adversarial debate, and final implementation contract for SILO PR #71 and the release-hardening work tracked by SILO #75.

Status: The complete B2+B3 server solution merged through PR #80 as b6ef7e430; the final asymmetric status-count regression merged through PR #81 as 04d3d316d. Final main Go CI and VulnCheck passed. Public documentation merged through silo.pgsty.com PR #7, and GitHub Pages plus Cloudflare Pages production deployment passed. Public EN/ZH routes were verified with HTTP 200. Issue #75 is closed as completed. No release tag, package, or container image was created by this closure.
Owner: pgsty/silo owns the server changes. This public design record belongs to pgsty/silo.pgsty.com. Console UI remains a separate deliverable.
Decision: accept the useful feature, preserve the contributor’s work, and block release until site-replication deletes, recovery, wildcard responses, and the narrow protocol follow-ups converge correctly.

The problem and solution in plain language

Before PR #71, SILO could set CORS only for the whole cluster. A browser application could not say, “allow this website to use bucket A, but not bucket B.” Standard S3 calls for reading, writing, and deleting a bucket’s CORS configuration existed as stubs and returned NotImplemented.

PR #71 added the missing feature. A bucket can now store its own allowed websites, HTTP methods, request headers, exposed response headers, and preflight cache time. Standard S3 clients can manage the configuration, and buckets without one keep the old global behavior.

The core feature works. The remaining problem appears when the same bucket is replicated between sites.

Imagine an administrator allows https://old.example.com, then removes that permission. SILO correctly deletes the rule on the first site. If another site temporarily misses that delete, the recovery process must later learn that “deleted at 10:05” is newer than “configured at 10:00.” The current code sometimes forgets the deletion time or replaces the source time with the time at which a peer received the message. Recovery can then mistake the old live rule for the newest state and restore it.

The fix is not a new replication system. A deleted configuration is represented by the data SILO already has:

configuration = absent
updated time  = time of DELETE

That pair is a deletion tombstone. The repair preserves the source timestamp for both PUT and DELETE, carries the timestamp even when no XML remains, and makes recovery use the same CORS apply path as normal peer delivery. Older events can no longer revive a newer deletion.

The direct cost is bounded: a CORS-specific distributed namespace lock and monotone state transition, focused ObjectLayer tests, strict wire validation, response-compatibility fixes, and operational documentation. There is no new storage field, dependency, feature flag, distributed clock, or general replication framework. The continuing cost is that SILO owns these tests and the bucket-CORS compatibility contract. Site replication still relies on synchronized wall clocks, as it already did.

CORS is not IAM authorization. A stale CORS rule does not grant an S3 permission that a principal lacks. It can, however, let a browser origin continue reading an already-authorized cross-origin response after an administrator intended to revoke that browser access. That is why convergence is a release blocker rather than cosmetic polish.

What PR #71 added

PR #71 replaced the inherited Bucket CORS stubs with a cohesive feature:

  • standard PutBucketCors, GetBucketCors, and DeleteBucketCors APIs;
  • Content-MD5 or supported checksum validation on PUT;
  • XML parsing and validation for origins, methods, allowed headers, exposed headers, rule IDs, and max age;
  • raw XML persistence in BucketMetadata with a CORS update timestamp;
  • per-bucket OPTIONS preflight handling and actual-response CORS headers;
  • the existing global CORS policy as a fallback only for buckets without a per-bucket configuration;
  • normal site-replication send, receive, initial-sync, status, and heal wiring;
  • unit, handler, middleware, metadata, and transport tests.

The local review rebased the feature onto the then-current main, built it, ran focused normal and race tests, full cmd tests, pinned lint, generated-file checks, compatibility checks, and a real minio-go smoke test. PUT, GET, DELETE, allowed and rejected preflights, Vary, and actual response headers all worked in the single-site path.

This evidence justified accepting the feature. It did not prove every failure-recovery path.

The reproduced convergence failures

Review-only tests against the real ObjectLayer reproduced the three failures below. A later release review also proved that payload-only status comparison kept different source-time barriers hidden, and that equal-timestamp conflicts depended on arrival and map-iteration order.

A newer DELETE can be ignored

The peer handler used the ordinary metadata Update and Delete methods. Those methods assign UTCNow() on the receiving site. If an older PUT arrives late, its locally generated arrival time can appear newer than a later source DELETE, so the DELETE is discarded.

An older PUT can revive a deletion

GetCorsConfig returns not-found and a zero timestamp once the live config is nil. The deletion time still exists in raw bucket metadata, but the handler cannot see it through that getter. A stale PUT therefore passes the staleness check and restores the rule.

Heal can select the stale live rule

SiteReplicationMetaInfo currently exports CorsConfigUpdatedAt only when CORS XML is present. After DELETE, the site reports nil configuration and zero time. A peer that still has the old XML reports a non-zero older time. Heal selects that old rule as “latest” and writes it back.

These are the same failure expressed at three seams: peer apply, metadata status, and recovery.

The intended state model

Per-bucket CORS needs only the state already present in BucketMetadata:

Logical state XML Timestamp Meaning
Never configured nil zero baseline; it is never transmitted or selected as a winner
Configured non-nil source PUT time live per-bucket rule
Deleted nil source DELETE time tombstone; newer than any earlier live rule

The selected register uses a deterministic total order:

1. source UpdatedAt
2. baseline < live < tombstone
3. equal-time live/live: lexicographic decoded payload bytes

Peer apply is a monotone join: it applies only a strictly greater state, so retry and duplicate delivery are idempotent. A tombstone wins an equal-time PUT/DELETE conflict, while two live values choose the same bytewise winner at every site. CreatedAt is not the baseline marker; it is only the bucket-lineage floor that rejects an event from an older bucket incarnation and emits a bucket-scoped diagnostic.

How the decision was made

Initial review

The first review agreed that the need was real and the single-site architecture was reasonable, but found that site replication emitted CORS events without completing every receive, status, and recovery path. The contributor added the missing wiring, checksum validation, wildcard/ID limits, cache variation, and focused tests.

A second runtime review confirmed the normal single-site and direct replication paths, then reproduced the tombstone and source-time failures above. The feature was close enough to accept, but not safe enough to release as complete.

Merge versus release

The maintainer chose to merge PR #71 and own the remaining hardening. This separated two decisions that are often confused:

  1. Is the contribution valuable and structurally sound enough to accept? Yes.
  2. Is the resulting feature ready to tag, package, publish, and deploy? Not until #75 closes.

The merge triggered full main CI, which passed. Release and Docker publication remain manual, independent gates.

Self-adversarial plan review

The first follow-up plan was intentionally comprehensive, then reviewed against four failure modes: overdesign, new problems introduced by the fix, failure to reuse existing infrastructure, and disproportionate maintenance cost.

That review removed or deferred:

  • a new general metadata-apply abstraction;
  • a custom wildcard matcher;
  • broad policy/tag/SSE/quota refactoring;
  • a multi-process site-replication test lab;
  • method-case, Unicode-ID, and trailing-XML strictness without differential evidence;
  • a no-Origin hot-path optimization that could alter existing Vary behavior;
  • vector clocks, a new tombstone field, and a global timestamp redesign.

Independent Claude Opus 5 reviews

Four read-only local Claude Code reviews used canonical claude-opus-5 at maximum effort. They moved the design from a timestamp-only patch to the final zero-baseline, deterministic C-prime register; required the distributed CORS lock and monotonic local barrier; made status and heal compare full state; and closed strict base64, semantic validation, cache, Vary, wildcard credentials, and initial-sync tombstone gaps.

The combined B2+B3 review then checked the strict parser, exact method and Unicode-ID contract, MaxAge presence, Origin-null forwarding marker, checksum classification, and replication/restart behavior together. It found a test helper conflict and the upgrade risk that a document accepted by a lenient development build could make all bucket metadata unavailable. The helper was corrected. Legacy-invalid CORS now leaves other bucket metadata readable, fails browser behavior closed, rejects new invalid saves, and remains repairable by a valid CORS PUT or DELETE.

Design goals and non-goals

Goals

  • make CORS PUT and DELETE converge under duplicate, delayed, reordered, and missed events;
  • preserve exact source timestamps on peer apply and heal;
  • let a newer nil tombstone beat an older live config;
  • avoid widening a configured bucket to global CORS on metadata failure;
  • align literal wildcard, credentials, exposed headers, and cache variation with S3 behavior;
  • correct the new CORS status count and the narrow validation gaps proven by existing matcher behavior;
  • keep the repair independently reviewable and reversible.

Non-goals

  • redesign every bucket metadata replication handler;
  • solve distributed clock skew or same-timestamp multi-writer conflicts globally;
  • add a new metadata schema, event log, queue, general metadata lock, or feature flag;
  • build a permanent multi-site process lab;
  • tighten unrelated XML or validation paths without evidence;
  • add Console UI;
  • mix historical Object Lock, tag, SSE, policy, quota, or versioning repairs into this branch.

Final repair design

Commit 1: preserve tombstones and source order

The CORS replication handler remains the single place for explicit peer CORS events.

Under a CORS-specific distributed namespace lock it will:

  1. require a non-empty bucket and non-zero source timestamp;
  2. require existing bucket metadata rather than fabricating it;
  3. read raw CorsConfigUpdatedAt, including a timestamp whose live config is nil;
  4. reject an event before the bucket lineage and ignore any state not strictly greater under the total order;
  5. strictly decode and validate a non-nil CORS payload or treat nil as DELETE;
  6. set CorsConfigXML and CorsConfigUpdatedAt directly from the source event, preserving the exact source barrier;
  7. persist through BucketMetadataSys.save, preserving the existing disk, cache, notification, and peer-node refresh path.

The legacy/default multi-field path may carry a non-nil CORS snapshot, so it uses the same lock, strict validation, and join; typed deletes continue through the CORS-specific handler. SiteReplicationMetaInfo always exports the source timestamp and encodes XML only when present. Status compares kind, decoded payload, and timestamp. Heal chooses the deterministic maximum and pushes it through the same transition, including when only the timestamp differs.

The zero baseline is deliberately not defaulted to bucket creation. Initial sync sends live and tombstone states but omits baseline. Local PUT and DELETE choose a timestamp strictly after max(UTCNow, CreatedAt, current barrier).

Commit 2: fail closed and match S3 responses

The middleware currently falls back to global CORS for every GetCorsConfig error. The repair distinguishes two cases:

  • true no-config: use the global policy, preserving existing behavior;
  • another metadata error on a request with Origin: log once and call the underlying S3 handler without global CORS headers.

This is fail-closed for browsers without converting a metadata problem into a new server-wide 500 contract. A failed preflight reaches the router’s ordinary non-CORS error response. Requests without Origin keep the existing middleware path; there is no speculative hot-path optimization.

Successful preflights also return configured Access-Control-Expose-Headers, which the S3 OPTIONS contract lists explicitly.

Origin matching will return the actual matched pattern. Response behavior is:

Matched origin element Access-Control-Allow-Origin Access-Control-Allow-Credentials
* * omitted
exact origin request origin true
pattern such as https://* request origin true

This matters when a rule contains both a specific origin and *: response semantics follow the first origin element that actually matched, rather than merely noticing that the rule contains a wildcard somewhere.

The three cache dimensions are set before the preflight match result, so both 200 and 403 responses vary by Origin, requested method, and requested headers.

Commit 3: narrow validation and status cleanup

The site summary increments TotalCorsConfigCount from the current site’s s.CorsConfig != nil, not from a cumulative count that may already include an earlier site.

Validation rejects:

  • an empty allowed origin;
  • ?, because the reused generic matcher treats it as a wildcard while S3 documents only a single * wildcard.

The implementation keeps the existing matcher and at-most-one-* rule. It does not change method case handling, ID character counting, or trailing XML behavior.

The handler test suite adds missing and mismatched Content-MD5 cases. That test exposed another concrete bug: the handler wrapped the checksum reader in an exact-ContentLength LimitReader, which returned EOF before the checksum wrapper could report a mismatched digest. After the existing positive and 64 KiB ContentLength guards, the handler now reads the wrapped request body to EOF directly. This preserves the shared validateLengthAndChecksum implementation and makes BadDigest observable without adding a second checksum path.

Commit 4: operator notes

The in-repository note records:

  • bucket CORS overrides rather than merges with global CORS;
  • DELETE restores global fallback;
  • an older binary does not enforce the new configuration;
  • an older binary rewriting bucket metadata may drop the unknown CORS fields;
  • an older peer harmlessly no-ops an unknown CORS event, then converges through heal after upgrade;
  • site replication still depends on synchronized clocks.

Public operational documentation remains a separate documentation-repository deliverable, represented by this record and any later task-oriented reference updates.

Test design

The tests exercise real state transitions without creating a permanent multi-process lab.

Test seam Required cases
Peer apply with ObjectLayer delayed PUT then newer DELETE; stale PUT after tombstone; duplicate delivery; exact source timestamps; missing metadata returns an error and creates no record
Transport-to-apply retain the existing JSON nil/non-nil round trip; feed at least one JSON-decoded event into the real peer handler
SiteReplicationMetaInfo nil config still carries the DELETE timestamp; pre-feature zero timestamp defaults to Created
Heal newer nil tombstone beats older live XML; local state becomes nil with the exact tombstone time
Middleware errors no-config uses global fallback; another metadata error gives actual and preflight responses no global CORS headers
Origin responses exact, literal *, patterned, and mixed-origin rules; credentials only when permitted
Preflight expose headers; allowed headers; max age; three Vary fields on success and rejection
Validation and handler empty origin, ?, missing Content-MD5, mismatched Content-MD5

The full admin-auth dispatch is not given its own integration fixture. It is a two-line switch already covered by compilation and review; the wire and real handler seams carry the meaningful state-machine risk. Startup’s concrete errBucketMetadataNotInitialized value is not frozen in a dedicated test; a representative non-not-found metadata error covers the middleware decision.

Rejected alternatives

Put CORS tombstone logic in the generic metadata merger

Rejected because it would give one field special nil, staleness, and early-return semantics inside a seven-field merge function. The CORS-specific handler already exists and is the smaller boundary.

Add a new timestamp-aware metadata abstraction

Rejected until at least two metadata types demonstrate identical requirements. A general helper today would encode assumptions about delete semantics that differ across policy, tag, SSE, object lock, quota, and versioning.

Add a physical tombstone field or event journal

Rejected because (nil config, DELETE timestamp) already represents the required state. A new schema increases downgrade and migration cost without adding information.

Replace wall clocks with a distributed ordering system

Rejected as disproportionate and inconsistent with existing site replication. Correctly preserving source time restores the current contract; it does not solve global clock skew.

Build a full multi-site test lab

Rejected because the failures are local state-machine defects and every important seam is directly testable in process. A lab would be slower, more brittle, and harder to diagnose.

Write a custom CORS wildcard matcher

Rejected because input validation can constrain the existing matcher to the S3-supported single-* language. Reimplementing matching creates more boundary cases than it removes.

Tighten every validation edge now

Rejected because uppercase-only methods, Unicode ID counting, and trailing-document rejection could change accepted inputs without evidence that they affect security or real client compatibility.

Fix every neighboring replication issue in the same branch

Rejected because shared-looking code does not prove shared semantics. Historical problems receive their own reproduction, issue, review, and release boundary.

Costs, benefits, and remaining risks

Benefits

  • standard S3 Bucket CORS works for browser applications and common SDKs;
  • buckets can use narrower origin policies than the cluster-wide fallback;
  • normal delivery, missed events, reorder, retry, and heal converge on the same state;
  • a revoked browser origin cannot be restored merely because a peer missed DELETE;
  • error handling cannot silently widen a configured bucket to global CORS;
  • wildcard and credentials behavior matches the established S3 client expectations.

Implementation and maintenance cost

The production changes remain local to bucket metadata timestamps, CORS peer apply/heal/status, CORS middleware, and CORS validation. The largest addition is regression coverage, because state convergence must be proven on both supported ObjectLayer test backends.

No new dependency, service, configuration key, storage field, background worker, or cross-repository server dependency is introduced. The ongoing cost is maintaining the S3 compatibility matrix, source-timestamp tests, and documentation.

Remaining risks accepted by design

  • wall-clock ordering assumes synchronized site clocks;
  • equal timestamps use a CORS-local deterministic tie-breaker rather than a global replication redesign;
  • mixed-version operation is unsupported for CORS writes; all sites must upgrade before the feature is enabled;
  • an old binary may ignore or later drop CORS metadata during rollback writes;
  • full Console management remains absent;
  • inherited replication defects outside CORS remain separate work.

These are visible constraints, not hidden claims of perfect parity.

Historical follow-ups kept separate

Adversarial review confirmed one unrelated defect in the existing initial-sync path: an Object Lock event is constructed with SRBucketMetaTypeObjectLockConfig but stores its payload in Tags instead of ObjectLockConfig. That requires a dedicated issue and fix.

Neighboring site summaries also use cumulative counters, and policy/tag/SSE/quota/versioning peer handlers may share source-time or tombstone weaknesses. The follow-up policy is:

  1. reproduce each behavior independently;
  2. open a focused issue with the affected metadata contract;
  3. do not modify it in the CORS branch;
  4. consider a shared helper only after at least two types require the same semantics.

This keeps historical cleanup honest without turning a bounded CORS repair into a site-replication rewrite.

An event earlier than local CreatedAt is ignored as belonging to an older bucket incarnation and logged once with a bucket-scoped key. Status keeps the mismatch visible; removing the floor would risk applying an old CORS grant to a newly recreated bucket.

Compatibility impact

Existing user or deployment Expected impact
No bucket CORS configured existing global CORS behavior remains
Single-site bucket CORS standard control plane and enforcement remain; response fidelity improves
Site replication without bucket CORS no behavioral change
Site replication with bucket CORS source ordering, DELETE, retry, and heal become reliable
Raw PUT caller must send the S3-required Content-MD5 or supported checksum
Older peer no-ops unknown CORS events until upgrade; heal converges afterwards
Downgrade bucket CORS is not enforced; metadata may be lost if an old binary rewrites the record
Console-only operator no CORS editor yet; use SDK, CLI, or S3 API

The stricter empty-origin and ? validation lands before any SILO release containing PR #71, so there is no released SILO bucket-CORS configuration population to migrate across that change.

Verification and release gates

The repair is complete only when all of the following are independently true:

  1. focused replication, middleware, validation, and handler tests pass;
  2. focused race tests pass;
  3. full cmd tests pass;
  4. go build ./..., pinned lint, generated-file checks, and compatibility checks pass;
  5. the standard minio-go PUT/GET/DELETE and preflight smoke test passes;
  6. an independent adversarial review finds no unresolved blocker;
  7. the follow-up server PR is committed, pushed, reviewed, and merged;
  8. its PR CI and the resulting main CI are green;
  9. the documentation build and bilingual link checks pass;
  10. release tag, packages, image publication, deployment, and production verification are completed as separate gates.

Until then, issue #75 remains open and no release or Docker image should advertise per-bucket CORS as release-ready.

Conclusion

Per-bucket CORS solves a real compatibility and browser-isolation problem, and PR #71’s core implementation was worth accepting. The remaining defect is not a reason to discard the feature; it is a reason to state the replication model precisely and finish it before release.

The final design preserves the source timestamp and nil tombstone through normal peer apply, status, and heal; fails closed without turning CORS metadata errors into a new S3 outage; fixes literal wildcard and cache behavior; and keeps validation changes evidence-based. It reuses the existing CORS handler, bucket metadata, save path, matcher, and ObjectLayer tests. It adds no general framework and does not pull unrelated historical repairs into the branch.

That is the minimum complexity needed to make the merged feature sufficient, safe, and maintainable.

13 - Per-Bucket CORS Wire Contract: Strict XML, Checksums, and Browser Responses

This document records the B3 protocol-hardening decision for per-bucket CORS after SILO PR #71 merged as e4e3007da. It covers only the S3 request body, validation, checksum, matching, and browser-response contract. Site-replication ordering, tombstones, heal, status counters, and generic metadata refactoring remain separate work under SILO #75.

Status: The strict B3 contract and B2 convergence solution merged through pgsty/silo PR #80, with the remaining explicit status-count matrix merged through PR #81. Final main Go CI and VulnCheck passed. The EN/ZH records merged through documentation PR #7; GitHub Pages and Cloudflare Pages production deployment passed, and all four public CORS design routes return HTTP 200. Issue #75 is closed. This closure did not create a release tag, package, or container image.
Decision: implement the strict B3 wire contract before the first SILO release containing per-bucket CORS. Do not normalize invalid input into validity, do not broaden the patch into site replication, and do not claim an overall release GO from local B3 evidence.

Why this is a release blocker

Bucket CORS is a standard S3 control plane. Its input is not merely configuration-shaped text: raw clients sign an exact XML request body, modern AWS SDKs attach a required payload checksum, SILO stores the accepted bytes verbatim, and GetBucketCors later returns those bytes to strict XML clients.

Three adversarial cases exposed holes in the merged implementation:

  1. a valid <CORSConfiguration> followed by a second XML root was accepted and stored;
  2. an ID containing exactly 255 Unicode characters was rejected because Go’s len(string) counted UTF-8 bytes;
  3. <AllowedMethod>get</AllowedMethod> was accepted because validation uppercased the value before checking the S3 enum.

These are server-side wire problems. The official AWS SDK models do not fully validate rule IDs or method strings on the client, and raw signed clients can always bypass typed SDK construction. The service must enforce the contract.

The second-root case is especially damaging. SILO stored the whole body, not just the first decoded element. A successful PUT could therefore make a later GET return a document with two roots, which standards-compliant XML clients reject.

Authoritative contract

The implementation uses current AWS documentation and generated SDK models as the protocol baseline:

  • PutBucketCors defines the XML root, the 64 KB document limit, Content-MD5 and SDK checksum headers, up to 100 rules, and the rule-match conditions: origin, method, and every requested header must all match.
  • CORSRule defines the uppercase method values and the inclusive 255-character ID limit.
  • Elements of a CORS configuration permits at most one * in each allowed origin or allowed header.
  • Testing CORS shows a successful preflight returning the matched rule’s full method list, requested allowed headers, exposed headers, credentials, and cache-variation headers.
  • S3 error responses defines MalformedXML for XML that does not validate against the S3 schema and BadDigest for a mismatched Content-MD5 or checksum value.
  • The generated AWS SDK for Go v2 PutBucketCors operation marks the request checksum as required. Its CORS types use an int32 MaxAgeSeconds and leave most semantic validation to the server.
  • The WHATWG Fetch Standard forbids sharing a credentialed response when Access-Control-Allow-Origin is *.

A read-only OPTIONS request to the public AWS landsat-pds bucket independently confirmed the current response behavior: a wildcard rule returned Access-Control-Allow-Origin: *, the full GET, HEAD method list, and no Access-Control-Allow-Credentials header.

Reproduction classification

Behavior Result Evidence and decision
second XML root accepted REAL parser, signed in-process handler, and real TCP SigV4 all accepted it before the fix
255 Unicode-character ID rejected REAL parser/Validate, signed handler, and real boto3 request reproduced SILO’s rejection; accepting 255 code points is based on AWS’s character wording and SDK model, not authenticated AWS PUT
lowercase method accepted REAL parser/Validate, signed handler, and real boto3 request all reproduced it
64 KiB boundary NOT REAL 65,536 bytes already passed and 65,537 failed; keep regression coverage
100-rule boundary NOT REAL exactly 100 already passed and 101 failed; keep regression coverage
first fully matching rule NOT REAL matching already fell through an earlier header-restrictive rule; preserve that behavior
checksum EOF bypass CONDITIONAL an in-memory reader could hide EOF from the checksum wrapper, while real TCP already rejected bad digests; remove the reader-dependent behavior anyway
empty and unknown XML members CONDITIONAL, resolved strictly AWS schema/error documentation supports rejection, but no authenticated AWS PUT black-box result was available
wildcard origin plus credentials REAL merged SILO echoed the origin and enabled credentials for *; live AWS and Fetch require * without credentials
Origin: null rewritten to wildcard REAL, found in final review inner forwarding middleware rewrote an explicitly matched null origin to * while retaining credentials; B3 now marks its response so the legacy rewrite skips it
negative MaxAge rejection CONDITIONAL, pre-existing retained because browser max-age is non-negative; no authenticated AWS PUT differential was available

Goals and non-goals

Goals

  • accept exactly one S3 CORS document element and only XML Misc after it;
  • enforce the documented 64 KiB, 100-rule, ID, method, wildcard, and MaxAge contracts;
  • verify Content-MD5 and modern SDK checksums independent of reader chunking behavior;
  • retain the first fully matching rule semantics;
  • return S3-compatible successful preflight and actual-request headers;
  • make every changed behavior reviewable through parser, Validate, signed handler, and real-client tests;
  • keep the exported compatibility manifest unchanged.

Non-goals

  • change site-replication delivery, tombstones, heal, or status accounting;
  • redesign global CORS fallback or metadata-error handling;
  • add Console UI;
  • introduce a general XML-schema framework;
  • validate arbitrary XML attributes or require one namespace spelling;
  • enforce cross-rule ID uniqueness without stronger current evidence;
  • refactor unrelated lifecycle, tagging, policy, SSE, quota, or versioning parsers;
  • commit, push, tag, publish an image, deploy, or claim production parity in this work item.

Alternatives considered

A. Patch only the three reported lines

This would add an EOF check, use a rune count, and remove method uppercasing. It is attractive but incomplete: it leaves unknown elements, duplicate singleton fields, empty numeric values, int32 overflow, the generic ? wildcard, reader-dependent checksum verification, and incorrect wildcard/preflight responses.

Rejected: too narrow for the explicitly reviewed B3 contract.

B. Normalize input into a canonical configuration

The server could uppercase methods, trim values, discard unknown elements, and keep only the first XML root. This is convenient for friendly clients but changes invalid signed wire input into a different valid configuration. It also preserves bytes that do not round-trip through GetBucketCors cleanly.

Rejected: S3 compatibility requires validation, not silent repair.

C. Add a strict, B3-specific wire representation

Decode into private XML wire structs that capture direct text, unknown elements, repeated singleton fields, and MaxAge presence. Convert into the existing public Config and Rule types only after the XML shape is valid. Keep semantic checks in Validate and matching helpers.

Selected: it is strict where evidence exists, order-independent, namespace-tolerant, local to CORS, and adds no exported compatibility symbol.

D. Validate every possible XML and header detail

This would enforce namespace URIs, reject every unknown attribute, validate every response header as an RFC token, and add cross-rule ID uniqueness.

Rejected for now: these constraints lack sufficient differential evidence and risk unnecessary incompatibility.

Final design

1. XML wire parser

ParseBucketCorsConfig decodes into private wire-only types:

  • CORSConfiguration is the only root;
  • root and rule levels reject non-whitespace direct character data;
  • unknown root, rule, and nested leaf elements are rejected;
  • ID and MaxAgeSeconds may occur at most once per rule;
  • list members remain repeatable and order-independent;
  • MaxAge text must parse as a signed 32-bit integer;
  • after the root closes, whitespace, comments, and processing instructions are allowed; another root, text, directive, or malformed token is rejected.

Namespace prefixes and the standard namespace declaration remain accepted because matching uses XML local names. Unknown attributes are not newly rejected. The existing Config and Rule XML tags remain for serialization compatibility, but production request and metadata parsing uses ParseBucketCorsConfig.

This parser also runs when stored bucket metadata is loaded. That is a deliberate pre-release choice: no SILO tag postdates PR #71, so there is no released per-bucket CORS population to migrate. A development build that previously stored malformed CORS XML makes the bucket’s entire metadata record unloadable—not only its CORS view—until the stored CORS document is replaced or deleted.

2. Semantic validation

Validate enforces:

  • one through 100 rules;
  • valid UTF-8 and no more than 255 Unicode code points in an ID;
  • at least one non-empty allowed origin and one method per rule;
  • methods exactly equal to GET, PUT, HEAD, POST, or DELETE;
  • no ? in an allowed origin or allowed header, because the inherited matcher treats it as a wildcard while S3 documents only *;
  • at most one * in each allowed origin and allowed header;
  • non-empty allowed and exposed header elements;
  • MaxAgeSeconds from zero through 2^31-1.

An empty ID remains allowed because ID itself is optional and current AWS documentation publishes no non-empty constraint. Cross-rule ID uniqueness remains outside this patch.

3. Matching

The generic matcher is replaced on this path by a small single-* matcher:

no *  -> exact match
one * -> prefix and suffix must both match; * may match zero bytes

Allowed-header matching remains case-insensitive, while the requested header spelling is preserved in the response. Method matching is case-sensitive after the S3 PUT path validates canonical stored values. Direct site-replication and heal writes in the merged base bypass that validation; they remain a separate integration requirement and can otherwise store a method that the B3 matcher will not execute.

MatchPreflight continues past a rule that matches origin and method but rejects one requested header. The selected rule is therefore the first rule that matches all three documented conditions. It also returns the exact origin element that matched and whether MaxAgeSeconds was present, preserving the difference between absent and explicit zero.

4. Request size and checksums

The handler keeps the existing positive Content-Length and 64 KiB guards. validateLengthAndChecksum still wraps the body with the shared checker, but the CORS handler now reads that wrapped body to EOF instead of placing another exact-length LimitReader outside it.

This makes checksum verification independent of whether the underlying reader returns the final bytes with or without io.EOF in the same call. A well-formed but mismatched Content-MD5 or full-header SDK checksum returns BadDigest; missing checksum material still returns the existing required-checksum error. The shared helper can classify malformed checksum syntax as missing, and this small-body path does not implement aws-chunked trailing-checksum decoding; those fidelity gaps remain outside B3. No second checksum implementation is added.

Modern boto3 traffic is a material compatibility gate because current botocore sends x-amz-sdk-checksum-algorithm: CRC32 plus x-amz-checksum-crc32, not Content-MD5, for this required-checksum operation.

5. Browser responses

The matched origin element controls the response:

Matched element Access-Control-Allow-Origin Access-Control-Allow-Credentials
* * omitted
null null true
exact origin request origin true
pattern such as https://* request origin true

A successful preflight returns:

  • the matched rule’s complete AllowedMethods list;
  • only the requested headers that the rule permits;
  • configured ExposeHeaders;
  • MaxAgeSeconds, including explicit zero;
  • the existing three successful-preflight Vary dimensions.

Actual requests keep their existing continue-through behavior and receive origin, credentials, expose, and Vary: Origin headers when a rule matches. A request-context marker prevents the inner legacy forwarding middleware from rewriting an explicitly allowed null origin to *; unmarked global responses retain their historical workaround. Because null is shared by sandboxed documents and file:// origins, operators should configure it only when credentialed access from all such contexts is intentional.

Allowed-origin elements are evaluated in document order. If a rule contains both a specific origin and *, place the specific origin first when that origin must retain reflected-origin credentials semantics.

The rejected-preflight body remains the existing bare 403 in this B3 patch. Producing the full AWS AccessForbidden XML shape and changing rejected-response cache/audit behavior require a separate wire decision rather than being smuggled into parser hardening.

Implementation map

Area Files Responsibility
parser and validation internal/bucket/cors/cors.go private wire structs, strict trailing-token check, rune/enum/wildcard/MaxAge validation, matching
parser tests internal/bucket/cors/cors_test.go, cors_adversarial_test.go roots, XML Misc, unknown/nested/duplicate members, boundaries, matching
PUT handler cmd/bucket-cors-handlers.go size/checksum gates, EOF consumption, S3 error mapping
signed handler tests cmd/bucket-cors-adversarial_test.go three reported cases, 64 KiB, 100 rules, MD5 and CRC32 positive/negative cases
browser responses cmd/api-router.go, cmd/generic-handlers.go matched origin semantics, null marker, full methods, expose, explicit zero max age
response tests cmd/bucket-cors-middleware_test.go exact/pattern/wildcard/null origins, first full rule, headers, methods, expose, max age, credentials

No site-replication source file belongs to this implementation boundary.

Test and evidence matrix

Layer Required evidence
parser second root/text/dangling close rejected; trailing whitespace/comment/PI accepted; unknown/nested/duplicate rejected
Validate 255 Unicode characters accepted, 256 rejected; lowercase and unsupported methods rejected; wildcard and empty-value cases
boundary exactly 64 KiB and 100 rules accepted; one byte/rule over rejected; MaxAge absent/zero/negative/int32 overflow
signed handler raw SigV4 PUT for all three reported failures; missing/bad MD5; valid/bad SDK CRC32
middleware first fully matching rule; wildcard, pattern, and null credentials; legacy unmarked null rewrite; full methods; requested headers; expose; explicit max age zero
focused race CORS package and CORS handler/middleware tests under the race detector
full local untagged and kqueue,dev full cmd; build; vet; pinned lint; generated/rebrand; diff check
real clients minio-go v7.3.1 PUT/GET/preflight/DELETE and boto3/botocore CRC32 PUT/GET/preflight/DELETE plus adversarial rejects
external behavior read-only OPTIONS against a public AWS bucket for wildcard, methods, credentials, and Vary

Adversarial review resolution

Claude Code Opus 5 ran at max effort against the evidence, implementation, and then this bilingual design plus the final code. The earlier implementation review returned GO. The publication review returned GO WITH FIXES, with no P0 or P1 and five P2 findings. After the accepted code and documentation changes, the same session returned GO with no P0–P2 finding.

Its non-blocking findings were independently adjudicated:

  • the one behavioral P2 was accepted: an explicitly allowed actual Origin: null now survives the inner legacy forwarding middleware;
  • the metadata-load blast radius and replication-validation exception are now stated precisely;
  • returning an interior MaxAge pointer is read-only and the selected rule was already an interior pointer; no new mutation occurs;
  • BadDigest is retained because the current AWS S3 error reference explicitly applies it to Content-MD5 or checksum mismatch;
  • malformed checksum syntax, trailing-checksum decoding, no-match Vary, full AccessForbidden XML, and outer-middleware audit behavior are recorded but remain outside B3;
  • non-UTF-8 XML declarations are not enabled because the S3 request syntax is UTF-8 and current SDKs emit UTF-8;
  • method whitespace remains invalid while integer whitespace is accepted according to their different XML lexical domains;
  • validating every exposed header as an RFC token is deferred without AWS differential evidence.

Compatibility and rollout

Existing use Effect
typed minio-go or boto3 CORS valid configurations continue to round-trip; modern CRC32 requests are verified
raw valid XML accepted up to the same size and rule limits
lowercase method now rejected instead of normalized
255 non-ASCII ID characters now accepted; more than 255 rejected
second root, unknown element, duplicate singleton, empty/overflow MaxAge now rejected as malformed XML
literal wildcard origin now returns * without credentials
patterned origin concrete request origin remains reflected with credentials
old development metadata containing malformed CORS XML the entire bucket metadata record may be unloadable until the CORS XML is replaced or deleted
site replication no B3 code change; its own convergence repair and tests remain separate

The strictness change lands before any tagged SILO version contains per-bucket CORS. That timing is the compatibility window. Once released, this wire contract becomes stable and future relaxations or tightenings require their own differential evidence.

Verification result and remaining gates

The final local implementation passed:

  • focused parser, Validate, signed handler, and middleware tests;
  • focused race tests for internal/bucket/cors and CORS cmd paths;
  • go test ./cmd -count=1 and the full kqueue,dev cmd lane;
  • go build ./... and go vet ./...;
  • golangci-lint 2.13.1 with zero issues;
  • generated-file, compatibility/rebrand, entrypoint, and diff checks;
  • real boto3/botocore 1.43.58 and minio-go v7.3.1 regressions against a freshly built local server;
  • final Claude Code Opus 5 max-effort review.

These results establish B3 IMPLEMENTATION GO only. Overall release remains blocked until the separate replication work is integrated, the server and documentation changes are committed and pushed, PR and merged-main CI pass, a release artifact is built, and deployment/production checks complete independently.

Conclusion

The final B3 design treats Bucket CORS as a signed S3 wire contract rather than a forgiving configuration file. It rejects malformed or noncanonical input before persistence, counts IDs as characters, validates modern SDK checksums, preserves documented first-full-rule selection, and emits browser-safe S3 responses.

The patch stays local to the CORS parser, validator, handler, matcher, response code, and tests. It adds no new service, schema, dependency, exported compatibility symbol, or site-replication refactor. That is the smallest complete solution supported by the protocol and live evidence.