Repository navigation
fix(calibration): refuse config names that escape the calibration dir - #141
Open
parvpatodia wants to merge 1 commit into
Open
parvpatodia wants to merge 1 commit into
parvpatodia wants to merge 1 commit into
Conversation
start_calibration passed config_file straight into the LeRobot device id, and
LeRobot builds the calibration path as calibration_dir / f"{id}.json", so a
name containing ../ pointed the calibration file outside the calibration
directory. Apply the same is_valid_robot_name guard the delete endpoint and
robot records already use, before any state changes.
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.
start_calibrationpassesconfig_filestraight into the LeRobot deviceid, and LeRobot builds the calibration path ascalibration_dir / f"{id}.json". A name containing../therefore points the calibration file outside the calibration directory. With lerobot 0.6.0,SO101Follower(SO101FollowerConfig(port=..., id="../../escaped", calibration_dir=d)).calibration_fpathresolves tod/../../escaped.json.The calibration-config delete endpoint (
server.py) and the robot-record helpers already reject these names withis_valid_robot_name. This applies the same guard at the start ofstart_calibration, before any status change or worker thread, and returns{"success": False, "message": "Invalid configuration name"}.Test:
test_calibration_manager_rejects_unsafe_config_name, parametrized over"",../escape,..,a/b,a\band" padded". It checks that no calibration starts. It fails 6/6 without the change. Full suite: 331 passed.ruff0.14.1 check and format are clean.