diff --git a/docs/guides/getting-started.md b/docs/guides/getting-started.md index 45eca76..1ff173e 100644 --- a/docs/guides/getting-started.md +++ b/docs/guides/getting-started.md @@ -43,6 +43,22 @@ make report # self-contained HTML reports in artifacts/reports/ make db-query NAME=02_benchmark_disagreement ``` +The local HTTP API serves those generated HTML files with +`GET /reports/index.html`. Its server-owned report root defaults to +`artifacts/reports`; set `TRADEFORGE_REPORT_ROOT` before starting `make api` +if the reports were generated into another directory. For example: + +```bash +TRADEFORGE_REPORT_ROOT=/srv/tradeforge/reports make api +``` + +The optional `directory` query parameter selects a **relative subdirectory** +within that configured root, for example +`/reports/index.html?directory=research/baseline`. It no longer selects an +arbitrary filesystem directory. Absolute paths, parent traversal and symlinks +that resolve outside the configured root are rejected. Only `.html` reports +are served; the server operator should keep other data outside that root. + `make demo` prints all four algorithms in both styles. The first two rows, with the `participation` column dropped for width: diff --git a/src/tradeforge/interfaces/api.py b/src/tradeforge/interfaces/api.py index 40627b5..71fbc1d 100644 --- a/src/tradeforge/interfaces/api.py +++ b/src/tradeforge/interfaces/api.py @@ -17,7 +17,8 @@ from __future__ import annotations -from pathlib import Path +import os +from pathlib import Path, PurePosixPath, PureWindowsPath from typing import Any from fastapi import FastAPI, HTTPException, Query @@ -34,6 +35,7 @@ DEFAULT_ARTIFACT_ROOT = Path("artifacts/runs") DEFAULT_SQL_DIR = Path("sql") +DEFAULT_REPORT_ROOT = Path("artifacts/reports") app = FastAPI( title="TradeForge", @@ -43,6 +45,10 @@ ), version="0.1.0", ) +# The server selects this root at startup; request parameters cannot change it. +app.state.report_root = Path( + os.environ.get("TRADEFORGE_REPORT_ROOT", str(DEFAULT_REPORT_ROOT)) +).resolve() class RunRequestBody(BaseModel): @@ -211,13 +217,24 @@ def run_query(name: str) -> dict[str, Any]: @app.get("/reports/{name}", response_class=HTMLResponse) def get_report( - name: str, directory: Path = Query(default=Path("artifacts/reports")) + name: str, + directory: str = Query( + default="", description="Relative subdirectory of the server report root" + ), ) -> HTMLResponse: - """Serve a generated HTML report by filename.""" - candidate = (directory / name).resolve() - root = directory.resolve() - if root not in candidate.parents and candidate != root: - raise HTTPException(status_code=400, detail="path escapes the report directory") + """Serve a generated HTML report within the server-configured output root.""" + relative_directory = _report_subdirectory(directory) + if name in {".", ".."} or any(character in name for character in ("/", "\\", ":", "\x00")): + raise HTTPException(status_code=400, detail="report name must be a filename") + if Path(name).suffix.lower() != ".html": + raise HTTPException(status_code=404, detail=f"no report named {name!r}") + root = Path(app.state.report_root) + try: + candidate = (root / relative_directory / name).resolve() + except (OSError, RuntimeError, ValueError) as exc: + raise HTTPException(status_code=400, detail="invalid report path") from exc + if not candidate.is_relative_to(root): + raise HTTPException(status_code=400, detail="path escapes the server report root") if not candidate.is_file(): raise HTTPException(status_code=404, detail=f"no report named {name!r}") return HTMLResponse(candidate.read_text(encoding="utf-8")) @@ -226,6 +243,21 @@ def get_report( # ------------------------------------------------------------------ helpers +def _report_subdirectory(directory: str) -> Path: + """Accept only relative components, including when Windows syntax reaches Linux.""" + normalized = directory.replace("\\", "/") + path = PurePosixPath(normalized) + if ( + path.is_absolute() + or PureWindowsPath(directory).drive + or ".." in path.parts + or ":" in normalized + or "\x00" in normalized + ): + raise HTTPException(status_code=400, detail="directory must be relative to the report root") + return Path(*path.parts) + + def _configs() -> dict[str, Any]: configs = load_configs(DEFAULT_CONFIG_DIR) validate_required(configs) diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index a275a78..0929e6c 100644 --- a/tests/unit/test_api.py +++ b/tests/unit/test_api.py @@ -10,6 +10,11 @@ from __future__ import annotations +import os +import subprocess +import sys +from pathlib import Path + import pytest pytest.importorskip("fastapi", reason="the API needs fastapi") @@ -18,6 +23,7 @@ from fastapi.testclient import TestClient from tradeforge.interfaces.api import app +from tradeforge.interfaces.reports import HtmlReportBuilder @pytest.fixture(scope="module") @@ -138,6 +144,111 @@ def test_a_non_positive_quantity_is_rejected(self, client): assert response.status_code == 422 +@pytest.fixture +def report_root(tmp_path, monkeypatch) -> Path: + root = tmp_path / "reports" + root.mkdir() + monkeypatch.setattr(app.state, "report_root", root.resolve()) + return root + + +class TestReportScope: + def test_report_root_is_selected_by_server_startup_configuration(self, tmp_path): + configured_root = tmp_path / "operator-reports" + rendered = HtmlReportBuilder(configured_root).index([]) + code = ( + "import sys; " + "from fastapi.testclient import TestClient; " + "from tradeforge.interfaces.api import app; " + "response = TestClient(app).get('/reports/index.html'); " + "assert response.status_code == 200; " + "sys.stdout.buffer.write(response.content)" + ) + result = subprocess.run( + [sys.executable, "-c", code], + env={**os.environ, "TRADEFORGE_REPORT_ROOT": str(configured_root)}, + check=True, + capture_output=True, + ) + assert result.stdout.decode("utf-8") == rendered.path.read_text(encoding="utf-8") + + def test_serves_a_generated_report_from_the_configured_root(self, client, report_root): + rendered = HtmlReportBuilder(report_root).index([]) + response = client.get(f"/reports/{rendered.path.name}") + assert response.status_code == 200 + assert response.text == rendered.path.read_text(encoding="utf-8") + assert response.headers["content-type"].startswith("text/html") + + def test_serves_relative_subdirectories(self, client, report_root): + rendered = HtmlReportBuilder(report_root / "research" / "baseline").index([]) + response = client.get("/reports/index.html", params={"directory": "research/baseline"}) + assert response.status_code == 200 + assert response.text == rendered.path.read_text(encoding="utf-8") + + def test_caller_cannot_select_an_arbitrary_readable_directory( + self, client, report_root, tmp_path + ): + private = tmp_path / "private" + private.mkdir() + (private / "secret.html").write_text("private document", encoding="utf-8") + response = client.get("/reports/secret.html", params={"directory": str(private)}) + assert response.status_code == 400 + assert "private document" not in response.text + + @pytest.mark.parametrize( + "directory", + [ + "../private", + "research/../../private", + r"..\private", + "/private", + r"\private", + r"C:\private", + "C:private", + r"\\server\share", + "research:stream", + "research\x00", + ], + ) + def test_rejects_traversal_and_absolute_paths_on_either_platform( + self, client, report_root, directory + ): + response = client.get("/reports/index.html", params={"directory": directory}) + assert response.status_code == 400 + + @pytest.mark.parametrize("name", [r"..\private\secret.html", "index.html:stream"]) + def test_report_name_cannot_contain_path_components(self, client, report_root, name): + response = client.get(f"/reports/{name}") + assert response.status_code == 400 + + @pytest.mark.parametrize("directory_link", [False, True]) + def test_symlinks_cannot_escape_the_configured_root( + self, client, report_root, tmp_path, directory_link + ): + private = tmp_path / "private" + private.mkdir() + (private / "secret.html").write_text("private document", encoding="utf-8") + target = private if directory_link else private / "secret.html" + link = report_root / ("linked" if directory_link else "secret.html") + try: + link.symlink_to(target, target_is_directory=directory_link) + except OSError as error: + if os.name == "nt" and error.winerror == 1314: + pytest.skip("Windows does not allow this user to create symlinks") + raise + response = client.get( + "/reports/secret.html", params={"directory": "linked" if directory_link else ""} + ) + assert response.status_code == 400 + assert "private document" not in response.text + + def test_non_html_files_are_not_served(self, client, report_root): + (report_root / "notes.txt").write_text("internal notes", encoding="utf-8") + response = client.get("/reports/notes.txt") + assert response.status_code == 404 + assert "internal notes" not in response.text + + class TestOpenApi: def test_the_schema_is_served(self, client): """A published schema is what lets a caller avoid guessing."""