Fix case-run path traversal

[lil-aditya]

• Closes [Bug] Path traversal in case-run delete/download routes can escape selected case directory #430
• Related Replace zf.extractall with safe_extractall #322, fix(backupCase): store relative paths in ZIP archive #331, Fix: calculate relative arcname for backupCase ZIP generation (fixes #403) #404, fix(api): implement robust zip extraction and path sanitization #419

Existing related work reviewed

• Issues/PRs reviewed: Replace zf.extractall with safe_extractall #322, fix(backupCase): store relative paths in ZIP archive #331, Fix: calculate relative arcname for backupCase ZIP generation (fixes #403) #404, fix(api): implement robust zip extraction and path sanitization #419
• Replace zf.extractall with safe_extractall #322 and fix(api): implement robust zip extraction and path sanitization #419 are related security/path-sanitization work, but they focus on ZIP upload/extraction paths in UploadRoute.py.
• fix(backupCase): store relative paths in ZIP archive #331 and Fix: calculate relative arcname for backupCase ZIP generation (fixes #403) #404 are related ZIP archive path fixes for backupCase, but they focus on archive entry layout/relative paths.
• None of the reviewed PRs appear to fix deleteCaseRun or DataFile download traversal through caserunname / file in API/Routes/DataFile/DataFileRoute.py.

Overlap assessment

• Classification: Partial thematic overlap, not duplicate.
• Overlapping items: Path traversal/path sanitization concerns.
• Why this is not duplicate/superseded: This PR fixes a different route and attack surface: deleteCaseRun, downloadDataFile, downloadFile, downloadCSVFile, and downloadResultsFile in DataFileRoute.py. The related PRs focus on ZIP extraction or backup archive generation in UploadRoute.py, so they do not cover sibling case traversal through case-run/download parameters.

Why this PR should proceed

• It closes a DataFile route traversal gap where user-controlled caserunname or file values could escape the selected case/run directory while still staying under Config.DATA_STORAGE.
• It keeps valid case-run deletion working while rejecting traversal payloads with 400 Invalid path.
• It adds focused regression tests for both valid behavior and blocked traversal attempts.

Summary

• What changed: Added _safe_child_path(...), used it to validate nested DataFile delete/download paths against the intended child directory, added PermissionError handling for invalid delete paths, and added regression tests in tests/test_datafile_guards.py.
• Why: The previous guard validated against the broad DATA_STORAGE root, which could still allow traversal from one case/run path into a sibling case under the same storage root.

Validation

• Tests added/updated (or not applicable)
• Validation steps documented
• Evidence attached (logs/screenshots/output as relevant)

.\venv\Scripts\python.exe -m pytest tests\test_datafile_guards.py -q

platform win32 -- Python 3.11.9, pytest-9.0.2, pluggy-1.6.0

collected 3 items

tests\test_datafile_guards.py ... [100%]

3 passed, 1 warning in 0.63s
The warning was only a local pytest cache permission warning for .pytest_cache; the tests passed.

[Copilot]

Pull request overview

This PR addresses a path traversal vulnerability in DataFile-related delete/download routes by validating user-controlled path segments relative to the intended case/run subdirectory (instead of only validating against the broad Config.DATA_STORAGE root).

Changes:

• Added a _safe_child_path(...) helper and applied it to deleteCaseRun and multiple download routes to prevent sibling-case traversal.
• Added PermissionError handling to return 400 Invalid path. for invalid/traversal attempts.
• Added regression tests covering both allowed deletion and blocked traversal attempts.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
API/Routes/DataFile/DataFileRoute.py	Introduces narrower path validation for delete/download endpoints via _safe_child_path and maps invalid paths to HTTP 400.
tests/test_datafile_guards.py	Adds regression tests proving traversal to sibling cases is blocked and normal deletion still works.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

_safe_child_path can raise a TypeError when all provided parts are None/"" because os.path.join() is called with an empty argument list. This can be triggered by an empty string input (e.g., caserunname == ""), resulting in a 500 instead of a controlled 400. Consider explicitly handling the “no effective parts” case (e.g., raise PermissionError / return a 400) or ensure callers validate non-empty segments before calling this helper.

Suggested change

	relative_path = os.path.join(*[part for part in parts if part not in (None, "")])
	effective_parts = [part for part in parts if part not in (None, "")]
	if not effective_parts:
	raise PermissionError("At least one non-empty path segment is required")
	relative_path = os.path.join(*effective_parts)

[Copilot]

deleteCaseRun does not validate that caserunname is non-empty. Because validate_json_fields only checks key presence, a request with caserunname: "" will reach _safe_child_path(case_results_dir, caserunname) and currently produce a server error (TypeError from os.path.join) instead of returning a 400. Add an explicit check for an empty caserunname (consistent with the download routes) or make _safe_child_path robust to empty segments.

Suggested change

	return jsonify({'message': 'No model selected.', 'status_code': 'error'}), 400
	return jsonify({'message': 'No model selected.', 'status_code': 'error'}), 400
	if not caserunname:
	return jsonify({'message': 'No case run selected.', 'status_code': 'error'}), 400

[Copilot]

Catching PermissionError and returning “Invalid path.” will also convert real filesystem permission failures (e.g., inability to delete due to OS ACLs/locks) into a 400 that looks like a traversal attempt. If you want to preserve debuggability, consider only mapping PermissionError originating from Config.validate_path/_safe_child_path to 400 and letting deletion-time permission errors surface as 5xx/appropriate error codes.

[SeaCelo]

Thanks for this. We're prioritizing other work right now, so converting this to draft for the time being. We'll revisit when we're ready to pick these up.