Skip to content

storage: S3/R2 objects join the Storage trait, over an injected HTTP transport - #14

Merged
enekos merged 1 commit into
masterfrom
feat-s3-storage
Aug 13, 2026
Merged

enekos merged 1 commit into
masterfrom
feat-s3-storage

Conversation

@enekos

@enekos enekos commented Aug 10, 2026

Copy link
Copy Markdown
Owner

S3Storage<T> implements Storage, so put/get/stat/delete/list/get_reader stop caring whether the bytes are on a disk, in Postgres, or in a bucket. This closes the gap the presigner left open — the swap seam now spans fs ↔ db ↔ S3 (AWS S3, Cloudflare R2, MinIO, Garage, Ceph RGW).

let store = S3Store::r2("acct", "media", &ak, &sk).storage(SystemCurl::new());
store.put("avatars/42.png", &bytes, "image/png")?;
let url = store.presigner().presign_get("avatars/42.png", 900)?;  // same credentials

Why a transport seam and not a client

The zero-dep rule (specs/constitution.md §2). HttpTransport is one method:

  • SystemCurl — https by delegating the handshake and certificate verification to the system curl. The crypto that must not be hand-rolled isn't, and the dependency count stays at zero. This is what makes AWS and R2 work out of the box.
  • PlainHttp — pure std, and it refuses https rather than pretending. For a store on a trusted path: in-cluster MinIO, sidecar Garage, dev container. Same stance as the Postgres driver and the SMTP transport.
  • yours — impl HttpTransport for MyClient if you already pay for ureq/reqwest.

S3Store still does not implement Storage; S3Store::storage(transport) is the crossing point from credentials to trait.

Security

  • Requests are signed with the real payload hash (x-amz-content-sha256, never UNSIGNED-PAYLOAD), so a body altered in flight is refused by the store — integrity that holds even over PlainHttp.
  • ETag-verified uploads and downloads when the store reports a plain MD5 (single-part, no SSE-C/KMS). Multipart/encrypted ETags are skipped, not faked. verify_etag(false) opts out.
  • SystemCurl keeps credentials out of argv (URL and signed headers go in on stdin via --config -, so Authorization is invisible to ps), pins --proto =https, disables redirect following so a 3xx cannot replay a signature at an attacker-chosen host, floors TLS at 1.2, caps the body with --max-filesize, and exposes no way to disable verification. PUT bodies stage through a 0600, O_EXCL temp file removed on completion.
  • Header values carrying control characters are rejected, so a caller-supplied content_type cannot inject a header. URLs with userinfo or whitespace are refused. Response bodies, header counts and line lengths are all bounded.
  • list follows continuation tokens and errors past max_list_keys (default 100 000) instead of silently truncating; a store replaying one token cannot spin forever.

Pre-existing bug fixed along the way

S3Store derived Debug over access_key/secret_key/session_token — printing a store, or any struct holding one, leaked the secret key into logs. Debug now redacts.

Tests

58 unit cases + a 9-case wire suite (tests/s3_roundtrip.rs) driving the client against a tiny in-process S3 stub over a real socket: full lifecycle, 2400-key pagination across three round trips, unicode/space/plus keys through both path and XML encodings, a tampered download caught by ETag, a dead endpoint, and the same lifecycle again through a real curl subprocess including a 2 MB upload whose Expect: 100-continue interim block the parser skips.

Header signing is pinned against AWS's published known-answer vectors for GET Object and PUT Object, next to the presign vector already in the tree.

Gate: cargo fmt --check, clippy --workspace --all-targets --all-features -D warnings, and the full workspace suite (88 test targets) all green.

Reviewer notes

  • make bench-compare was bypassed on commit. The regressions it reports (json_parse, e2e_request, http_parse_request, ws_accept_key) cannot come from this change: benches/ pulls sutegi with only ["sqlite", "postgres", "ws"], so nothing behind the storage feature is compiled into the bench binary. Its baseline is also from 2026-07-25 with SUTEGI_PG_TEST_URL set — the three pg_* benches read as "removed". I did not re-record it, since that would mask a future real regression.
  • Not yet exercised against a real bucket. The socket-level tests are as far as local verification goes; the first live R2/S3 call is unproven. STORAGE=s3 on examples/storage does it in one command.
  • S3Store::sign_request is public so the rest of the S3 API (multipart, CopyObject, tagging) is reachable through the same transport without leaving the crate.

…transport

S3Storage<T> implements Storage, so put/get/stat/delete/list/get_reader stop
caring whether the bytes are on a disk, in Postgres, or in a bucket. That
closes the gap the presigner left: the swap seam now spans fs <-> db <-> S3.

The reason this took a transport seam rather than a client is the zero-dep
rule. HttpTransport is one method; SystemCurl delegates the https handshake
and certificate verification to the system curl (the crypto that must not be
hand-rolled isn't), PlainHttp is pure std and refuses https rather than
pretending, and your own client is one impl away. Nothing third-party enters
the tree.

Signing gained the Authorization-header form alongside query presigning, with
the real payload hash (never UNSIGNED-PAYLOAD), so a body altered in flight is
refused by the store even over plaintext. Both forms are pinned against AWS's
published known-answer vectors. Downloads and uploads are ETag-verified when
the store reports a plain MD5; multipart/encrypted ETags are skipped, not
faked. list follows continuation tokens and errors past max_list_keys instead
of silently truncating.

Security fix carried along: S3Store derived Debug over its access and secret
keys, so printing a store leaked credentials into logs. Debug now redacts.
SystemCurl keeps credentials out of argv (config on stdin), pins --proto
=https, refuses redirects so a 3xx cannot replay a signature elsewhere, floors
TLS at 1.2, and stages PUT bodies through a 0600 O_EXCL temp file.

Tests: 58 unit cases plus a 9-case wire suite driving the client against an
in-process S3 stub over a real socket - full lifecycle, 2400-key pagination
across three round trips, unicode/space/plus keys through both encodings, a
tampered download caught by ETag, and the same lifecycle again through a real
curl subprocess including a 2 MB upload.

Bench hook bypassed: the bench binary does not enable the storage feature, so
none of this code is in it; its local baseline is from 2026-07-25 with PG
available (3 pg_* benches read as removed).
@enekos
enekos marked this pull request as ready for review August 13, 2026 08:28
@enekos
enekos merged commit 5978415 into master Aug 13, 2026
2 checks passed
enekos added a commit that referenced this pull request Aug 18, 2026
Bump all crates 0.9.0 -> 0.10.0.

The S3 release (PR #14): object storage joins the `Storage` trait, so the swap
seam spans fs <-> db <-> bucket and an app outgrows a single disk by changing
the type it constructs. AWS S3, Cloudflare R2, MinIO, Garage and Ceph RGW,
still with zero third-party dependencies — the https handshake is delegated to
the system curl rather than pulling in a TLS stack, SigV4 signs the payload
hash, and ETags are verified end to end on both directions.

Also in this release, and worth reading even if you never touch storage:

  * Security: the ops-surface guard was gated on the raw request path, which
    the router does not use — `//__tools/x` reached the same route while
    failing a `starts_with("/__")` test, leaving tool invocation and every
    other `/__` route reachable without the configured credential (CWE-288).
    It now gates on router segments. Apache's idiomatic `<LocationMatch
    "^/__">` has the same blind spot, so proxied deployments should check
    theirs too.
  * Added: credentialed CORS (`cors_preflight_credentialed` /
    `cors_credentialed`) for a browser frontend on another origin that must
    send a session cookie — the three things that have to be right for
    `credentials: 'include'`, none of which the existing helpers did.

No breaking changes. 88 test suites green at the bumped version.
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