feat(compat): bump baton wire-compat pin to 6.1.0 (physical_path) - #102
Merged
Conversation
Upstream released 6.0.1 and 6.1.0 since our 6.0.0 pin. Re-pin to 6.1.0 and record the two known gaps against it as tracked issues rather than letting the drift go unnoticed: - #100: baton-rs's genQuery builder has no bound on WHERE/SELECT condition count, the same OOB-write class upstream patched in 6.0.1 (PRs #337/#338). - #101: 6.1.0 added a physical_path field to replicate JSON objects that baton-rs doesn't emit yet. Updates BATON_COMPAT_VERSION, Cargo.toml's description, and every README/SESSIONS.md reference to the compat target.
…ery bounds check) Closes the two gaps found against the 6.1.0 re-pin instead of just claiming it: - #100: shim_query_add_where/add_select now reject once the query's condition/select list reaches iRODS's MAX_NUM_CONDITIONS, instead of writing past the fixed-size sqlCondInp/selectInp arrays -- the same OOB-write class upstream baton patched in 6.0.1 (wtsi-npg/baton#337, #338). Reachable from unbounded caller-supplied JSON via metaquery's timestamps/access arrays. add_select gained a Result return (25 call sites updated); new integration test drives 2000 access entries and asserts a clean in-band error. - #101: added SHIM_COL_DATA_PATH -> COL_D_DATA_PATH through the usual shim declare/implement/mirror path, Replicate.physical_path, and wired it into fetch_replicates. Extended the --replicate integration test and the Replicate round-trip unit test. Bumped the partisan pin (58b56da -> 1356610d2a, devel HEAD) to pick up partisan PR #349's test_replicas assertion, which requires physical_path whenever client_version() >= 6.1.0 -- the thing this work needed exercised in CI. Extendo's pin is untouched; it has no physical_path work yet.
jmtcsngr
changed the title
Confirmed against iRODS's actual source that inxIvalPair_t/inxValPair_t are pointer-based structs and addInxIval/addInxVal dynamically grow their backing storage via malloc -- there is no fixed capacity to write past. Upstream baton's MAX_NUM_CONDITIONS (which the reverted fix incorrectly assumed was an iRODS constant, and which doesn't exist in iRODS headers at all -- confirmed by a CI compile failure) guards a fixed-size buffer in baton's *own* C code, not the shared client library baton-rs's shim calls directly. baton-rs never had this vulnerability. Reverts the shim bounds check, GenQuery::add_select's Result return (and the 25 call-site updates), and the now-invalid integration test. #101 (physical_path) is unaffected and stays in place. Issue #100 closed with a correction rather than left stale.
jmtcsngr
changed the title
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
1 participant
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
Upstream baton released 6.0.1 (2026-05-19) and 6.1.0 (2026-07-02) since our 6.0.0 pin. Re-pins to 6.1.0 -- as an actual compat feature, not a version-string bump: the real gap found against the new pin is implemented and tested before
BATON_COMPAT_VERSIONmoves.physical_pathfield to every replicate JSON object (Add physical path reporting for replicates wtsi-npg/baton#339). AddedSHIM_COL_DATA_PATH->COL_D_DATA_PATHthrough the shim,Replicate.physical_path: String, wired intofetch_replicates. Extended the--replicateintegration test and theReplicateround-trip unit test.inxIvalPair_t/inxValPair_tare pointer-based andaddInxIval/addInxValdynamically grow viamalloc-- there's no fixed capacity to overflow. Upstream'sMAX_NUM_CONDITIONSis baton's own self-imposed limit guarding a fixed buffer in baton's own C code, not a property of the shared iRODS client library baton-rs's shim calls directly. The attempted fix was reverted; issue closed with a correction.Also bumped
.github/scripts/partisan-pin(58b56da->1356610d2a, devel HEAD) to pick up partisan PR #349'stest_replicasassertion, which requiresphysical_pathwheneverclient_version() >= 6.1.0-- exactly the thing this PR needs exercised in CI. Extendo's pin is untouched; no physical_path work exists there yet (checked thev3.1.0..v3.2.0diff and a repo-wide search), and extendo uses plainencoding/json.Unmarshalwith noDisallowUnknownFields, so the new field is safely ignored there regardless.Test plan
unit-tests.yml) green across the 4.2.7 / 4.3.4 / 4.3.5 matrixpartisan-tests.ymlgreen against the bumped pin, including the newphysical_pathassertion intest_replicasextendo-tests.ymlunaffected