From 836698cfc20eaecfd1bd8b6ac0b07bc34c1a2bae Mon Sep 17 00:00:00 2001 From: Soeren Wacker Date: Fri, 17 Jul 2026 14:48:14 +0200 Subject: [PATCH] fix: reject path traversal in dataset names on load/delete/exists Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01FTZQT3bQzqTBXQXvDsk4w9 --- .../repositories/filesystem_dataset.py | 15 ++++++- .../test_dataset_repository.py | 40 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/metaseed/repositories/filesystem_dataset.py b/src/metaseed/repositories/filesystem_dataset.py index b9bc7e7..0e96a05 100644 --- a/src/metaseed/repositories/filesystem_dataset.py +++ b/src/metaseed/repositories/filesystem_dataset.py @@ -44,7 +44,20 @@ def _ensure_dir(self: Self) -> None: self._dir.mkdir(parents=True, exist_ok=True) def _get_path(self: Self, name: str) -> Path: - """Get the file path for a dataset.""" + """Resolve the on-disk path for a dataset, rejecting unsafe names. + + The name is interpolated straight into a filename, so a value like + ``../../etc/passwd`` or an absolute path would escape the datasets + directory. This is the single choke point shared by save/load/delete/ + exists, so validating here protects every operation rather than only + the save path. + + Raises: + ValueError: If the name is not a valid dataset name. + """ + error = self.validate_name(name) + if error: + raise ValueError(error) return self._dir / f"{name}.json" def list(self: Self) -> list[DatasetInfo]: diff --git a/tests/test_repositories/test_dataset_repository.py b/tests/test_repositories/test_dataset_repository.py index b39d681..ef4d0fb 100644 --- a/tests/test_repositories/test_dataset_repository.py +++ b/tests/test_repositories/test_dataset_repository.py @@ -149,6 +149,46 @@ def test_delete_nonexistent(self, repo): """Should return False for nonexistent.""" assert repo.delete("nonexistent") is False + @pytest.mark.parametrize( + "evil_name", + [ + "../secret", + "../../etc/passwd", + "sub/../../escape", + "/etc/passwd", + "..", + ], + ) + def test_load_rejects_path_traversal(self, repo, tmp_path, evil_name): + """load() must not read files outside the datasets directory. + + The name becomes a filename, so an unvalidated ``../secret`` (or an + absolute path) would escape the datasets dir. Every read/delete path, + not only save, must reject such names. + """ + outside = tmp_path / "secret.json" + outside.write_text('{"name": "secret", "profile": "x", "version": "1"}') + + with pytest.raises(ValueError): + repo.load(evil_name) + + def test_delete_rejects_path_traversal(self, repo, tmp_path): + """delete() must not unlink files outside the datasets directory.""" + victim = tmp_path / "victim.json" + victim.write_text("{}") + + with pytest.raises(ValueError): + repo.delete("../victim") + assert victim.exists() # untouched + + def test_exists_rejects_path_traversal(self, repo, tmp_path): + """exists() must not probe files outside the datasets directory.""" + outside = tmp_path / "probe.json" + outside.write_text("{}") + + with pytest.raises(ValueError): + repo.exists("../probe") + def test_exists(self, repo): """Should check existence correctly.""" assert not repo.exists("test")