Rework push OID checks and align Syfon APIs - #250
matthewpeterkort wants to merge 26 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09886249d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Migrate git-drs to Syfon's current generated API and client packages. Preserve credentials, scope lookup, copy, delete, and download behavior across the updated boundaries. Pin Docker integration to the tested Syfon commit, including the corrected bulk hash response contract.
91d2f44 to
e0e5e4e
Compare
Use Syfon client operations for reusable transfer and bucket behavior. Delete dormant reconciliation and test-only paths, consolidate shared helpers, and remove generated coverage artifacts.
matthewpeterkort
changed the title
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e5bd271fb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return fmt.Errorf("size must not be negative for %q", file.Path) | ||
| } | ||
|
|
||
| dst := filepath.Join(root, rel) |
There was a problem hiding this comment.
When a component beneath PullOptions.Root is a symlink, a lexically safe path such as link/file.bin passes safeRelativePath, but MkdirAll and the download then follow root/link and write outside the requested root. This defeats the API's root-containment check and can overwrite arbitrary writable files when the output tree is shared or attacker-controlled; resolve the destination with component-wise symlink rejection immediately before opening it.
Useful? React with 👍 / 👎.
| localObj, err := localdrsobject.ReadObject(gitrepo.DRSObjectsPath, oid) | ||
| if err == nil && localObj != nil && firstAccessURL(localObj) != "" { | ||
| urlOIDs = append(urlOIDs, oid) |
There was a problem hiding this comment.
When --force-upload is used and the existence endpoint reports an ordinary OID as present, this selection fetches the scoped record only if a local sidecar has an access URL. In a normal clone existingByHash therefore remains empty, so ensureMetadataRegistered retains a newly generated deterministic object ID and uploadFileForObject passes that ID to syupload.Upload; if the existing scoped record was imported under a different DID, the forced upload targets an unrelated DID and can fail or leave the existing record unmodified. Include all present OIDs in the targeted lookup when force-upload is enabled.
Useful? React with 👍 / 👎.
| oid := strings.ToLower(strings.TrimSpace(strings.TrimPrefix(info.Oid, "sha256:"))) | ||
| if oid != "" { | ||
| hashes[oid] = struct{}{} |
There was a problem hiding this comment.
For a path containing a DRS-URI pointer created by add-ref, info.Oid is the URI while info.SHA256 contains the optional content checksum. Adding the URI to this filter means copyRecordMatchesIncludedSHA256 cannot match any source record's SHA-256, so copy-records --include-path silently copies zero records even when the selected pointer has a valid checksum; use info.SHA256 for DRS pointers and reject entries that have no usable SHA-256.
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Pin the published Syfon audit-fix commit for backend E2E. Port the Calypr kind suite to build git-drs from the current checkout and Syfon from the shared pin.
This branch has not been deployed
1 participant
Summary
git drs rmremoves Git pointers; it does not delete remote records or payloads.Verification
go test -race ./...go vet ./...go test ./internal/transfer -run '^TestPushLookupFindsReusableRecordOutsideTargetScope$' -count=1