fix: reject path traversal in dataset names on load/delete/exists

[sorenwacker]

Security fix — path traversal via dataset name

FilesystemDatasetRepository interpolates the dataset name straight into a filename (filesystem_dataset.py _get_path: self._dir / f"{name}.json"). The allowlist validate_dataset_name (^[a-zA-Z0-9][a-zA-Z0-9_-]*$) was applied only in save() — load(), delete(), and exists() skipped it. So a name like ../secret or an absolute path escaped the datasets directory.

Reachable unauthenticated over HTTP

POST /api/datasets/load takes the name via Body(..., embed=True) (ui/routes/api.py), so / and .. arrive unencoded, and the web app has no authentication. manager.load_dataset(name) → FilesystemDatasetRepository.load(name) → _get_path.

Confirmed with a contained PoC (before the fix)

load('../secret_credentials') -> LOADED FROM OUTSIDE THE DIR. entities = [{'api_key': 'SUPER_SECRET_TOKEN'}]
delete('../victim')           -> returned True | victim exists AFTER: False   (file outside dir REMOVED)
save('../evil')               -> correctly blocked (allowlist works, just wasn't applied to reads/deletes)
Path('/base/datasets') / '/etc/passwd.json' -> /etc/passwd.json               (absolute name replaces base)

Targets must end in .json (the suffix is appended), so this is arbitrary-.json read/delete rather than fully arbitrary — still a real information-disclosure and destructive primitive for a self-hosted instance.

The fix

Move the existing name validation into _get_path — the single choke point shared by save/load/delete/exists — so every operation is protected, not just save. list() uses glob and is unaffected. The load route already maps ValueError → HTTP 400, so invalid names now return 400 instead of escaping.

After the fix, all four PoC operations raise ValueError and valid names (my_study-01) still save/load/delete/exist normally.

Tests

Added to TestFilesystemDatasetRepository:

• test_load_rejects_path_traversal — parametrized over ../secret, ../../etc/passwd, sub/../../escape, /etc/passwd, ..
• test_delete_rejects_path_traversal — asserts an outside victim.json is not unlinked
• test_exists_rejects_path_traversal

tests/test_repositories + test_ui/test_datasets.py + test_ui/test_dataset_manager.py: 88 passed. ruff + strict mypy clean.

Not included (follow-ups)

• repositories/file.py FileEntityRepository.from_dataset_name has the identical base_dir / f"{name}.json" pattern. It currently has no callers in src/, so it is not reachable — but it should get the same guard before it is ever wired to input.
• The web UI/API has no authentication at all; this fix removes the traversal but not the broader exposure. Worth a separate discussion.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FTZQT3bQzqTBXQXvDsk4w9