Add GeoJSON and improve API#113
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| content_type, _ = mimetypes.guess_type(str(file_path)) | ||
| return FileResponse(file_path.open("rb"), content_type=content_type or "application/octet-stream") | ||
| return FileResponse( | ||
| file_path.open("rb"), |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
This autofix suggestion was applied.
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
In general, the fix is to normalize and validate any filesystem path derived from user input before using it, and to ensure it is contained within an intended root directory. For this view, we should: (1) ensure the input is just a filename (no directory separators), (2) resolve the combined path (settings.SIMULATION_OUTPUT + filename) to an absolute/normalized path, and (3) verify that the resolved path is indeed inside settings.SIMULATION_OUTPUT. Only then should we use it with FileResponse.
Concretely, within SimulationFileView.get in opendrift_leeway_webgui/leeway/views.py, we should:
- Build a
base_dir = Path(settings.SIMULATION_OUTPUT).resolve()once per request. - Reject any input that is not a “flat” name using a simple, robust check like
if "/" in path or "\\" in path: raise Http404, which is clearer and avoids relying onPath(path).parent. - Construct
file_path = (base_dir / path).resolve(). - Ensure
file_pathis underbase_dir; for Python 3.9+ we can usefile_path.is_relative_to(base_dir), and otherwise fall back to atry/except ValueErroraroundfile_path.relative_to(base_dir). If it is outside, raiseHttp404. - Keep the remaining logic intact: verify the file exists, extract
uuid_str = file_path.stem, check ownership, and then stream the file.
These changes occur only inside the SimulationFileView.get method body; no new imports are necessary because we already import Path and settings. Functionality from the caller’s perspective remains the same (serving per-user simulation outputs by filename), but traversal and out-of-root access are now blocked in a way that satisfies CodeQL and is secure.
| @@ -140,12 +140,20 @@ | ||
| """ | ||
| Serve the requested simulation file if the user owns it. | ||
| """ | ||
| file_path = Path(settings.SIMULATION_OUTPUT) / path | ||
| base_dir = Path(settings.SIMULATION_OUTPUT).resolve() | ||
|
|
||
| # Only allow a flat filename — no directory traversal | ||
| if Path(path).parent != Path("."): | ||
| # Only allow a flat filename — no directory traversal or path separators | ||
| if "/" in path or "\\" in path: | ||
| raise Http404 | ||
|
|
||
| file_path = (base_dir / path).resolve() | ||
|
|
||
| # Ensure the resolved path is inside the simulation output directory | ||
| try: | ||
| file_path.relative_to(base_dir) | ||
| except ValueError: | ||
| raise Http404 | ||
|
|
||
| if not file_path.is_file(): | ||
| raise Http404 | ||
|
|
svenseeberg
committed this autofix suggestion 3 months ago.
|
GeoJSON export works: |
svenseeberg
force-pushed
the
fix/linting
branch
4 times, most recently
from
6dfcc37 to
4315695
Compare
svenseeberg
force-pushed
the
fix/linting
branch
from
55520ac to
578120a
Compare
| raise Http404 | ||
|
|
||
| base_dir = Path(settings.SIMULATION_OUTPUT).resolve() | ||
| file_path = (base_dir / path).resolve() |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Copilot Autofix
AI 3 months ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
|
best would be to use autoliniting via pre-commit with ruff as linter |
3 participants
No description provided.