feat: validate the sha256 after pulling the blob #168
Conversation
|
""" WalkthroughThe update adds SHA-256 digest validation for blobs and manifests pulled from a remote repository. After downloading content, the code computes its SHA-256 hash and checks it against the expected digest. If the digests do not match, an error is returned. A helper function for digest validation is introduced. The test suite is enhanced to use realistic file contents and corresponding digests for more accurate testing of blob fetching. Additionally, retry delay parameters for backend processes are increased. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Backend
participant RemoteRepo
Client->>Backend: Request to pull blob/manifest
Backend->>RemoteRepo: Fetch content
RemoteRepo-->>Backend: Return content stream
Backend->>Backend: Read content & compute SHA-256 hash
Backend->>Backend: Validate computed hash vs expected digest
alt Digest matches
Backend-->>Client: Success (content available)
else Digest mismatch
Backend-->>Client: Error (digest validation failed)
end
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🧰 Additional context used🧬 Code Graph Analysis (1)pkg/backend/pull.go (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (11)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/backend/pull.go (1)
231-246: Well-structured validation functionThe validateDigest function is well-structured with appropriate checks and clear error messages. The validation covers empty digest check, hash length check, and digest comparison.
Two minor suggestions:
- Consider extracting "sha256:" as a constant to avoid hardcoding the algorithm name
- Consider using encoding/hex package for the hash formatting
+ const digestAlgorithmPrefix = "sha256:" func validateDigest(digest string, hash []byte) error { if digest == "" { return fmt.Errorf("digest is empty") } if len(hash) != sha256.Size { return fmt.Errorf("invalid hash length") } - if digest != fmt.Sprintf("sha256:%x", hash) { - return fmt.Errorf("actual digest %s does not match the expected digest %s", fmt.Sprintf("sha256:%x", hash), digest) + computedDigest := digestAlgorithmPrefix + hex.EncodeToString(hash) + if digest != computedDigest { + return fmt.Errorf("actual digest %s does not match the expected digest %s", computedDigest, digest) } return nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/backend/pull.go(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (go)
🔇 Additional comments (5)
pkg/backend/pull.go (5)
25-29: Good job on organizing importsThe imports are now properly grouped and sorted, making the code more readable and maintainable.
140-141: Well-implemented hash verification approachUsing
io.TeeReaderto calculate the hash while reading the content is an efficient approach as it avoids having to read the content twice.
191-196: Good integrity check implementationAdding digest validation after pushing the content ensures data integrity. This is a critical security feature for artifact distribution systems.
212-214: Consistent application of hash verificationThe same TeeReader approach used in pullIfNotExist is correctly applied here for consistency.
221-226: Good error handling for digest validationThe error handling is thorough and includes aborting the progress bar, which provides good feedback to the user in case of corruption.
chlins
force-pushed
the
feat/pull-validate-sha256
branch
from
2500e59 to
9793166
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/backend/fetch_test.go(4 hunks)pkg/backend/pull.go(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/backend/pull.go
🧰 Additional context used
🪛 GitHub Actions: Lint
pkg/backend/fetch_test.go
[error] 84-84: Error return value of w.Write is not checked (errcheck)
[error] 86-86: Error return value of w.Write is not checked (errcheck)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
pkg/backend/fetch_test.go (2)
44-51: Great test setup with realistic content!Using explicit file contents and computing their corresponding digests with
godigest.FromString()makes the tests more realistic and ensures proper validation of the newly added SHA-256 digest validation functionality. This approach is better than using static mock values.
65-67: Good update to use accurate content sizes and digests in the manifestThe test now correctly uses the computed digests and actual file sizes in the manifest, making the test more realistic and aligned with the SHA-256 validation functionality introduced in this PR.
Also applies to: 73-75
chlins
force-pushed
the
feat/pull-validate-sha256
branch
from
9793166 to
0adf2bf
Compare
Signed-off-by: chlins <chlins.zhang@gmail.com>
chlins
force-pushed
the
feat/pull-validate-sha256
branch
from
0adf2bf to
36fe73c
Compare
This pull request enhances the integrity verification process in the
pkg/backend/pull.gofile by introducing digest validation for downloaded blobs. It also includes minor import reorganization for better readability. Below are the key changes grouped by theme:Integrity Verification Enhancements:
io.TeeReaderin bothpullIfNotExistandpullAndExtractFromRemotefunctions to calculate the hash of the downloaded content while reading. [1] [2]validateDigestfunction to ensure the computed hash matches the expected digest, improving data integrity checks. This validation is applied after content processing in both functions. [1] [2]Code Organization:
Summary by CodeRabbit