Restrict API reports to the configured output root

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:
 

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)

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."""