Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `RLTaskEnv.step` publishes the *terminal* observation in `info["observations"]["raw"]["obs"]` instead of the episode's first one (off-policy truncation bootstraps in clean_rl SAC/TD3 and FastTD3 read it).
- IsaacGym and PyBullet reported `joint_pos_target` in native DoF order while `joint_pos` is in sorted-name order; both now use `get_joint_names(sort=True)` (completes #12).
- `ParallelSimWrapper`: a worker that died during handler construction or `launch` surfaced as a bare `EOFError`/`ConnectionResetError` from the handshake and left the other workers running; the handshake now raises the worker's own traceback, `close()` tolerates dead workers, and a failed constructor tears the pool down.
- `hf_util.check_and_download_single`: a `roboverse_data/...` path evaluated from a working directory that is not the parent of `ROBOVERSE_DATA_DIR` was reported as a path-traversal attempt; the error now names the CWD / `ROBOVERSE_DATA_DIR` mismatch and where the asset already is. `test_check_and_download_single_falls_back_to_private_roboverse_data` no longer depends on the caller's `ROBOVERSE_DATA_DIR`.
- MuJoCo: `<size memory="512M">` is reserved by default; humanoid + mesh scenes no longer die with
`mj_stackAlloc: out of memory` (get_started/10_mount_camera.py).
- `hf_util`: a symlinked `roboverse_data` is no longer refused as path traversal; concurrent
Expand Down
5 changes: 5 additions & 0 deletions metasim/test/test_hf_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

from __future__ import annotations

import os

import pytest

import metasim.utils.hf_util as hf_util
Expand Down Expand Up @@ -50,6 +52,9 @@ def fake_file_exists(repo_id, filename, repo_type):
def fake_hf_hub_download(repo_id, filename, repo_type, local_dir):
download_calls.append((repo_id, filename, repo_type, local_dir))

# LOCAL_DIR is resolved from ROBOVERSE_DATA_DIR at import time; pin it to the CWD-relative
# default so the test does not depend on the caller's environment.
monkeypatch.setattr(hf_util, "LOCAL_DIR", os.path.abspath("roboverse_data"))
monkeypatch.setattr(hf_util.os.path, "exists", lambda path: False)
monkeypatch.setattr(hf_util.hf_api, "file_exists", fake_file_exists)
monkeypatch.setattr(hf_util, "hf_hub_download", fake_hf_hub_download)
Expand Down
23 changes: 23 additions & 0 deletions metasim/test/test_hf_util_local_dir.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,26 @@ def _fake_find(relpath_posix, is_optional_file=False):
hf_util.check_and_download_single(str(real_dir / "robots" / "g1" / "meshes" / "pelvis.STL"))
assert "Refusing to fetch" not in str(excinfo.value)
assert seen["relpath"] == "robots/g1/meshes/pelvis.STL"


@pytest.mark.general
def test_cwd_mismatch_error_names_the_cause(monkeypatch, tmp_path):
"""A ``roboverse_data/...`` path evaluated from the wrong CWD must explain the CWD /
ROBOVERSE_DATA_DIR mismatch (and where the asset already is), not accuse the caller of traversal."""
from metasim.utils import hf_util

data_dir = tmp_path / "shared_data"
(data_dir / "robots").mkdir(parents=True)
(data_dir / "robots" / "franka.urdf").write_text("<robot/>", encoding="utf-8")
elsewhere = tmp_path / "elsewhere"
elsewhere.mkdir()
monkeypatch.chdir(elsewhere)
monkeypatch.setattr(hf_util, "LOCAL_DIR", str(data_dir))
monkeypatch.setattr(hf_util.hf_api, "file_exists", lambda *a, **kw: pytest.fail("HfApi reached"))

with pytest.raises(ValueError) as excinfo:
hf_util.check_and_download_single(os.path.join("roboverse_data", "robots", "franka.urdf"))
msg = str(excinfo.value)
assert "working directory" in msg and str(elsewhere) in msg
assert str(data_dir / "robots" / "franka.urdf") in msg
assert "ROBOVERSE_DATA_DIR" in msg and "traversal" not in msg
32 changes: 28 additions & 4 deletions metasim/utils/hf_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,33 @@ def extract_texture_paths_from_mdl(mdl_file_path: str) -> list[str]:
return texture_paths


_RELATIVE_DATA_PREFIX = "roboverse_data" + os.sep


def _outside_local_dir_message(filepath: str, relpath: str) -> str:
"""Explain why ``filepath`` cannot be fetched into ``LOCAL_DIR``.

Asset paths in configs are ``roboverse_data/...`` and are opened by the simulator backends
relative to the *current working directory*; ``ROBOVERSE_DATA_DIR`` only moves where downloads
land. So a relative path that does not resolve under ``LOCAL_DIR`` is almost always a CWD /
``ROBOVERSE_DATA_DIR`` mismatch, not a malicious ``..`` — say so, and point at the asset when it
already exists in the configured directory.
"""
if not os.path.isabs(filepath) and filepath.startswith(_RELATIVE_DATA_PREFIX):
in_local_dir = os.path.join(LOCAL_DIR, filepath[len(_RELATIVE_DATA_PREFIX) :])
found = f" The asset exists at {in_local_dir!r}." if os.path.exists(in_local_dir) else ""
return (
f"Cannot fetch {filepath!r}: asset paths are relative to the working directory "
f"({os.getcwd()!r}), which is not the parent of LOCAL_DIR ({LOCAL_DIR!r}).{found} "
f"Run from the repository root, or symlink ./roboverse_data to LOCAL_DIR "
f"(ROBOVERSE_DATA_DIR only changes where downloads are stored)."
)
return (
f"Refusing to fetch {filepath!r}: resolves outside LOCAL_DIR ({LOCAL_DIR}) via relative-path "
f"traversal ({relpath!r})."
)


def check_and_download_single(filepath: str):
"""Check if the file exists in the local directory, and download it from the huggingface dataset if it doesn't exist.

Expand All @@ -145,10 +172,7 @@ def check_and_download_single(filepath: str):
# outside LOCAL_DIR — refuse it rather than send the escaped path
# to the HF API. Optional files warn-and-skip; required files raise.
if relpath.split(os.sep, 1)[0] == "..":
msg = (
f"Refusing to fetch {filepath!r}: resolves outside LOCAL_DIR "
f"({LOCAL_DIR}) via relative-path traversal ({relpath!r})."
)
msg = _outside_local_dir_message(filepath, relpath)
if is_optional_file:
log.warning(msg + " Skipping optional file.")
return
Expand Down
Loading