Skip to content

fix: validate digest-addressed manifest uploads - #8

Open
KAJdev wants to merge 1 commit into
mainfrom
zeke/sls-305-security-manifest-digest-spoof-serverless-registry
Open

fix: validate digest-addressed manifest uploads#8
KAJdev wants to merge 1 commit into
mainfrom
zeke/sls-305-security-manifest-digest-spoof-serverless-registry

Conversation

@KAJdev

@KAJdev KAJdev commented Jul 20, 2026

Copy link
Copy Markdown

Validates digest-addressed manifest uploads against the SHA-256 computed from the request body before any manifest object is written. Mismatches return the OCI-compatible DIGEST_INVALID client error, preserving the integrity guarantee of digest-pinned pulls.

Addresses SLS-305.

@KAJdev
KAJdev requested a review from deanq July 20, 2026 23:33
@celine-runpod
celine-runpod requested a review from jebenexer July 29, 2026 17:25

@jebenexer jebenexer left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM — validation is placed correctly (after digest computation, before any JSON parse or R2 write), returns a proper OCI DIGEST_INVALID error, and the test verifies both the rejection and that neither the forged nor the real digest gets written to R2.

Two non-blocking notes:

  1. Case sensitivity: comparison is case-sensitive against a sha256: prefix. Worth a quick test with an uppercase-hex digest to confirm/document that it's rejected as malformed rather than silently normalized — but rejecting is the safe default either way.
  2. Direct-upload path bypass (pre-existing, not introduced here): finishUpload's state.direct path (r2.ts ~834-861) trusts the client-provided digest when R2 checksum metadata is missing, which is a separate integrity gap from what this PR fixes. Filing a follow-up ticket for that rather than blocking this PR on it.

Approving — this closes a real digest-spoofing gap for the standard upload path.

@jebenexer

Copy link
Copy Markdown

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