[mikemikimike]

Summary

• Namespace LeRobot source caches by the resolved immutable repository SHA.
• Prevent reused output directories from mixing artifacts across revisions.
• Preserve same-SHA cache reuse and manifest provenance.

Closes #294

Validation

• ruff check, ruff format --check, ty check
• focused LeRobot converter tests
• full pytest suite: 1250 passed, 8 skipped

[github-actions]

👋 Hi @mikemikimike — thank you so much for your first contribution to HFlow!

A maintainer will review your pull request as soon as possible. In the meantime:

• ⭐ Star the repo: https://github.com/Hebbian-Robotics/hflow
• 💬 Join our Discord: https://discord.gg/vacepQvjmg
• 📖 Check CONTRIBUTING.md for the validation commands CI will run
• 📣 Share the project with your network

💡 Tip: one open pull request per contributor at a time. Issues with an assignee are taken; everything else is fair game.

We are excited to have you here and appreciate your help making the project better! 🙌

[kstonekuan]

Thanks, and welcome. The fix is the right one-liner and the test earns its place: routing tag-a and branch-a to the same sha and asserting the marker file survives proves reuse and separation in the same assertion, rather than just checking two directory names differ.

One thing before merge, and it's a consequence of your change rather than something you inherited.

Until this PR the cache path was a constant. Now it ends in a string that came off the network, and nothing checks its shape. _hf_repo_info only requires a non-empty str:

resolved_revision = repository_information.get("sha")
if not isinstance(resolved_revision, str) or not resolved_revision.strip():

So DoD 5 ("the cache remains entirely below output_dir / "_lerobot_cache"") is stated but not enforced:

sha='8f2c1ab9...'                      -> /tmp/probe-328-out/_lerobot_cache/8f2c1ab9...   below: True
sha='../../../../tmp/probe-328-escaped' -> /tmp/probe-328-escaped                          below: False
sha='/tmp/probe-328-absolute'           -> /tmp/probe-328-absolute                         below: False

To be clear about the size of this: it needs Hugging Face to return a hostile sha, so it is not a live path traversal and I don't want the commit message calling it one. It matters because the DoD names it, and because your change is what turned a constant path segment into a remote-controlled one.

Refuse a sha that isn't a commit hash where it's resolved, next to the check above. A commit sha is hex, so something like this is enough:

if not re.fullmatch(r"[0-9a-f]{7,64}", resolved_revision):
    raise ValueError(f"Hugging Face returned a malformed commit sha for {repo_id}@{revision}")

Then a case in your existing parametrized style asserting ../../evil is refused. Your test stubs _hf_repo_info, so this one wants to go at the real function.

Gate is otherwise clean on your head: 1252 passed / 6 skipped.

[kstonekuan]

LGTM, merging. DoD 5 is enforced now rather than assumed:

ACCEPTED '8f2c1ab9d4e5f60718293a4b5c6d7e8f90a1b2c3'  -> below _lerobot_cache: True
ACCEPTED '8f2c1ab'                                   -> below _lerobot_cache: True
refused  '../../../../tmp/probe-328-escaped'
refused  '/tmp/probe-328-absolute'
refused  '..'
refused  'C:/windows'
refused  '8f2c1ab9/../../../etc'

Putting it in _hf_repo_info rather than at the join is the right place: the sha is wrong there, not just unsafe as a path, and every future caller of the resolved revision gets it. The match="malformed commit sha" in your test matters too, since a bare pytest.raises(ValueError) there would also pass on the pre-existing empty-sha refusal.

I merged current main in and pushed as 62c3156, because main moved a long way under this and #326 landed in both files you touch. That surfaced a real interaction, which is why I mention it rather than just fixing it quietly:

FAILED tests/test_lerobot_converter.py::test_hf_repo_info_valid_json_still_resolves

#326's fixture uses {"sha": "abc123"}, six hex characters, one short of your {7,64} minimum. Your constraint is right and the fixture was the unrealistic part, so I changed it to abc1234. Nothing about either change is wrong; it's just a collision only visible on the merged result, which is the argument for the bound you picked being a real one.

Full gate on the merged result: clean, 1340 passed / 6 skipped.

One small thing, no action needed. Your namespacing test uses sha-a and sha-b, which the validator you just added would reject. It passes because that test stubs _hf_repo_info wholesale, so it never sees them. Fine as is, but if you touch it again, hex fixtures would keep the two tests telling the same story.