feat: [CODE-4993] refactor the upload flow of Git LFS

[sanket-deepsource]

No description provided.

[deepsource-io]

DeepSource Code Review

We reviewed changes in b4c0308...c279b6b on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Focus Area: Security

Security   Reliability   Complexity   Hygiene

Feedback

Path handling consistency

• Two of the security issues come from how paths are constructed, leading to traversal and arbitrary deletion risks.
• Given how careful the upload lifecycle is, it’s worth holding path joining to the same standard so the integrity guarantees apply both to where data lives and how it’s reached.

Receiver semantics vs. abstraction cleanliness

• The mixed pointer/value receivers and the Delete “warning only” behavior both create subtle differences in how storage backends behave.
• Your abstraction is nicely disciplined; aligning method semantics (receivers and delete behavior) will help keep that contract predictable across implementations.

Code Review Summary

Analyzer	Status	Updated (UTC)	Details
Go		Jun 20, 2026 5:38p.m.	Review ↗

[deepsource-io]

Type "FileSystemStore" has both value and pointer receivers

(Go's FAQ)[https://go.dev/doc/faq#methods_on_values_or_pointers] recommends
that method receivers should be consistent. If some of the methods of the type
must have pointer receivers, the rest should too, so the method set is
consistent regardless of how the type is used. This is because value and
pointer receivers have different
method sets.

[deepsource-io]

Type "FileSystemStore" has both value and pointer receivers

(Go's FAQ)[https://go.dev/doc/faq#methods_on_values_or_pointers] recommends
that method receivers should be consistent. If some of the methods of the type
must have pointer receivers, the rest should too, so the method set is
consistent regardless of how the type is used. This is because value and
pointer receivers have different
method sets.

[deepsource-io]

Expect directory permissions to be 0750 or less

Excessive permissions granted when creating a directory. This warning is
triggered whenever permission greater than 0750 is given.

[sanket-deepsource]

@deepsourcebot review

[deepsource-io]

`fmt.Sprintf` path join enables filesystem traversal

Unvalidated srcPath/dstPath can resolve outside c.basePath. If any caller passes attacker-influenced values, arbitrary file relocation or overwrite becomes possible.
Use filepath.Clean, reject absolute and .. segments, then verify resolved paths keep the c.basePath prefix before renaming

[deepsource-io]

`fmt.Sprintf` path join enables arbitrary file deletion

filePath is not normalized or constrained to the blob root before deletion. A crafted traversal path could delete unintended host files when upstream validation is bypassed.
Use filepath.Clean, reject absolute/.. paths, and ensure the resolved target remains under c.basePath before deletion

[deepsource-io]

`expectError` branch skips `errorType` validation

The expectError path only verifies err != nil. A wrong failure reason still passes, weakening regression detection for move error contracts.

Add errorType to negative cases and assert errors.Is(err, test.errorType) before returning

[deepsource-io]

`srcObj.Delete` warning-only handling leaves duplicate objects

Move logs srcObj.Delete errors but still returns success. That breaks move semantics and can leave both source and destination objects present, causing stale temporary blobs and inconsistent state for downstream workflows.

Return a wrapped error when srcObj.Delete fails, or implement compensating rollback by deleting dstObj before returning an error