lfs: verify content hash and prevent object overwrite#8166
Merged
Conversation
Prevent cross-repository LFS object overwrite by: 1. Using O_CREATE|O_EXCL to never overwrite existing files on disk. If the file already exists, it was previously hash-verified, so we return the existing size and let the caller create the DB record. 2. Computing SHA-256 of uploaded content and rejecting uploads where the hash does not match the claimed OID. Reported-by: KKC73 References: GHSA-gmf8-978x-2fg2 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the local LFS storage backend to prevent cross-repository object overwrites and to ensure uploaded content matches the claimed SHA-256 OID, addressing GHSA-gmf8-978x-2fg2.
Changes:
- Add SHA-256 verification during
LocalStorage.Uploadand reject uploads whose content hash doesn’t match the OID. - Prevent overwriting existing LFS objects by using atomic create semantics and returning existing object size on duplicates.
- Extend LFS storage tests to cover OID mismatch cleanup and duplicate upload behavior, and document the fix in the changelog.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| internal/lfsutil/storage.go | Adds OID/content verification and uses exclusive create to prevent overwrites. |
| internal/lfsutil/storage_test.go | Updates/extends tests for hash verification, cleanup on mismatch, and duplicate upload behavior. |
| CHANGELOG.md | Notes the security fix in the development changelog. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Write to a temp file and rename into place after hash verification, so the final path always contains a complete file even under concurrent uploads. This also avoids the deferred cleanup accidentally deleting a pre-existing file it did not create. - Handle ErrOIDMismatch in the upload route to return 400 instead of 500. - Check file close error before publishing. - Use t.TempDir() in tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
unknwon
commented
Feb 8, 2026
- Lowercase error message for ErrObjectNotExist. - Use a dedicated .tmp directory under the storage root for temp files instead of mixing them with object files. - Add subdir under t.TempDir() for test clarity. - Capitalize OID in test names and move comment inline. - Rename "oid mismatch" test to "valid OID but wrong content". Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
unknwon
commented
Feb 8, 2026
Add [lfs] TEMP_PATH config option (default: data/tmp/lfs) for storing temporary files during upload hash verification, following the existing pattern of [repository.upload] TEMP_PATH. Clean up the LFS temp directory on startup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
unknwon
commented
Feb 8, 2026
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
unknwon
commented
Feb 8, 2026
If os.Rename fails because the target already exists (concurrent upload won the race), treat it as success since both uploads have verified content. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
4 tasks
unknwon
added a commit
that referenced
this pull request
Feb 19, 2026
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
O_CREATE|O_EXCLto atomically refuse writing to an existing file path. If the file already exists (same OID uploaded by another repo), the existing size is returned so the caller can still create the DB record linking the new repo to that OID.Test plan
TestLocalStorage_Uploadupdated with matching OID/content pairtask lintpassestask testpasses (lfsutil)References: GHSA-cj4v-437j-jq4c
🤖 Generated with Claude Code