Skip to content

fix: harden CORS and replication request trust - #101

Merged
Vonng merged 7 commits into
mainfrom
codex/fix-cors-replication-trust
Sep 1, 2026
Merged

fix: harden CORS and replication request trust#101
Vonng merged 7 commits into
mainfrom
codex/fix-cors-replication-trust

Conversation

@Vonng

@Vonng Vonng commented Sep 1, 2026

Copy link
Copy Markdown
Member

Contribution Licensing (no CLA, inbound=outbound, DCO required)

This project does not use a CLA; contributions are accepted inbound=outbound.
Every commit in this PR carries a DCO Signed-off-by trailer.

Description

  • make the pre-authentication per-bucket CORS lookup resident-memory-only, with fail-closed startup/read-error behavior and no metadata I/O or cache insertion on misses;
  • derive replication trust once after authentication from an exact marker and the corresponding ReplicateObject/ReplicateDelete permission, strip untrusted internal headers after signature verification, and route all privileged option/encryption/object-lock/event behavior through the context decision;
  • count site-replication CORS only when the full live state has canonical base64, valid XML, and a non-zero source timestamp;
  • verify ReplicateObject/ReplicateDelete before returning replication validity-probe success, and place probe objects under the active rule prefix;
  • evaluate concurrent Snowball extraction trust independently per entry using immutable request snapshots, including streaming-trailer and bucket-default-encryption coverage.

Motivation and Context

The old CORS middleware could turn arbitrary valid-looking bucket path segments into pre-authentication metadata reads and persistent cache entries. Separately, client-controlled replication headers acquired internal meaning in SSE-C reads, source ETag/MTime preservation, object lock, checksums, events, and deletes before one receiver-wide trust decision existed.

The status loop then counted malformed or timestamp-less CORS payloads as configured, and replication setup probes returned before checking the permissions required at runtime. Adversarial review also found that the first trust implementation mutated shared request authorization state across concurrent Snowball entries; this PR closes that race and the related prefix-authorization bypass.

Refs #77 for the CORS status-accounting portion.

How to test this PR?

Passed locally on macOS/arm64 with Go 1.27.0:

  • focused CORS status and Put/Delete replication-trust tests;
  • focused tests under -race;
  • Snowball per-entry trust and bucket-default SSE tests, including a genuine streaming unsigned CRC32 trailer, under -race;
  • go build ./...;
  • go vet ./cmd ./internal/bucket/object/lock/;
  • go test ./cmd -count=1 (final run: 131.426s).

A real local TLS two-site run of the core trust commit also passed SSE-C single PUT, 12 MiB multipart, SSE-C CopyObject, and delete-marker replication with matching hashes, ETags, sizes, version IDs, and key MD5s.

Claude Code Fable 5 reviewed the design and implementation; Claude Code Opus 5 performed repeated adversarial acceptance reviews. Final verdict: GO, no P0/P1/P2 findings.

Compatibility impact

  • No S3 wire schema, object format, bucket metadata format, encryption format, or client command changes.
  • Untrusted internal replication headers are ignored after signature verification and the request follows ordinary S3 semantics.
  • Replication validity probes now actually require ReplicateObject/ReplicateDelete. Misconfigured target credentials fail during setup instead of failing continuously at runtime.
  • Prefix-scoped rules probe within rule.Prefix(). Empty-prefix behavior and the documented bucket/* policy remain unchanged.
  • An older source binary still probes the reserved root key, so prefix-scoped target credentials require all participating source sites to run the fixed validation logic before configuration-time checks fully align.
  • Startup CORS cache misses remain fail-closed; post-initialization misses fall back to global CORS. A real bucket whose metadata failed to load remains fail-closed.
  • Rollback leaves no new persisted schema. Source timestamps and existing metadata remain readable by the previous binary.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Optimization (provides speedup with no functional changes)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • All commits are signed off (git commit -s) per the DCO
  • Fixes a regression: Snowball per-entry replication trust introduced in 938603458
  • Unit tests added/updated
  • make verifiers passes
  • Relevant package tests and build pass
  • Compatibility and rollback impact documented
  • Internal documentation updated
  • Public documentation update opened in pgsty/silo.pgsty.com

Keep pre-authentication CORS lookups resident-only so attacker-controlled path segments cannot trigger metadata I/O or grow the metadata cache. Preserve fail-closed behavior for startup, load failures, invalid metadata, and the internal namespace.

Centralize replication request trust after authentication, distinguish general replication from replica-only privileges, and gate SSE-C ciphertext handling, source metadata, object-lock bypasses, event suppression, delete semantics, and replica status on the appropriate permission. Add least-privilege, multipart, PostPolicy, CORS amplification, and compatibility regressions.

Signed-off-by: Feng Ruohang <rh@vonng.com>
Count only valid live CORS states in per-site summaries. Treat baselines and tombstones as absent, and diagnose malformed payloads or missing source timestamps.\n\nRefs: #77

Signed-off-by: Feng Ruohang <rh@vonng.com>
Evaluate ReplicateObject or ReplicateDelete before returning the no-op validation response, so underprivileged target credentials fail during replication setup instead of at runtime.

Signed-off-by: Feng Ruohang <rh@vonng.com>
Evaluate PutObject and ReplicateObject permissions with immutable per-entry request snapshots during concurrent Snowball extraction. Preserve the first API error without sharing mutable handler state, and cover prefix-scoped trust under the race detector.

Signed-off-by: Feng Ruohang <rh@vonng.com>
Place synthetic permission-check objects under each enabled rule's effective prefix, so least-privilege target policies are validated against the namespace they will actually replicate.

Signed-off-by: Feng Ruohang <rh@vonng.com>
Snapshot per-entry requests after applying bucket encryption defaults but before streaming trailers are consumed. Keep authorization failures fatal while retaining Snowball ignore-errors behavior for object-lock failures.

Signed-off-by: Feng Ruohang <rh@vonng.com>
Accept the new CORS test routes, resident getter, and replication header literals in the rebrand guard. Apply gofumpt, context-first helper ordering, and spelling fixes required by CI.

Signed-off-by: Feng Ruohang <rh@vonng.com>
@Vonng

Vonng commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

Final acceptance update:

  • Local focused/race tests, full go test ./cmd, build, vet, real TLS two-site replication: PASS.
  • Claude Code Opus 5 repeated adversarial acceptance: GO, no P0/P1/P2.
  • Rebrand guard and golangci lint fixed; final automatic PR checks are all green, including release validate, cross-compile, race, cmd/internal, DCO, analysis, build/vet, and lint.

No merge, tag, package, image, deployment, or production verification has been performed.

@Vonng
Vonng merged commit fe9dd90 into main Sep 1, 2026
9 checks passed
@Vonng
Vonng deleted the codex/fix-cors-replication-trust branch September 2, 2026 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant