Skip to content

fix(s3): verify Content-MD5 on PutObject - #302

Merged
ArnavBalyan merged 1 commit into
openlake-project:mainfrom
stanleys12:osc/putobject-verify-content-md5-and-return-242f
Oct 24, 2026
Merged

ArnavBalyan merged 1 commit into
openlake-project:mainfrom
stanleys12:osc/putobject-verify-content-md5-and-return-242f

Conversation

@stanleys12

Copy link
Copy Markdown
Contributor

PutObject now checks the Content-MD5 header. If the body doesn't match the digest, it returns 400 BadDigest and nothing gets stored. If the header isn't a base64 16-byte digest, it returns 400 InvalidDigest.

Motivation

put_object ignored Content-MD5, so a body corrupted in transit got stored and acked. DeleteObjects already checks the header.

Changes

  • s3/error.rs: add BadDigest and InvalidDigest, both mapped to 400.
  • handlers/objects.rs: decode the optional content-md5 header and pass the expected digest to Engine::put.
  • engine.rs: compare the expected digest to the MD5 we already compute while draining the body, before the version is promoted. EC staging gets cleaned up by the existing error path.

UploadPart isn't covered.

Testing

  • New engine test put_checks_content_md5_before_commit (matching digest succeeds, mismatch fails and leaves no object).
  • cargo fmt --check and cargo clippy --workspace --all-targets -- -D warnings are clean.
  • cargo test --workspace on macOS: openlake_kv_client fails to link its pyo3 cdylib, so I excluded it. Everything else passes except lock_server::tests::refresh_matches_uid_and_extends_lease. It's timing based and also fails on main here (3/3 runs).

Related issues

Related to #155

Written with Claude Code.

PutObject ignored the Content-MD5 header, so a body corrupted in
transit was stored and acknowledged. Decode the header in the handler
(400 InvalidDigest if it is not a base64 16-byte digest) and compare
it to the MD5 the engine already computes while draining the body.
The check runs before the version is promoted, so a mismatch returns
400 BadDigest and leaves no object behind; EC staging is swept by the
existing error path.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

@ArnavBalyan ArnavBalyan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks for the changes

@ArnavBalyan
ArnavBalyan merged commit 9b54aba into openlake-project:main Oct 24, 2026
5 checks passed
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.

2 participants