diff --git a/frontend/src/app/app.spec.ts b/frontend/src/app/app.spec.ts index 52f761337..75753d688 100644 --- a/frontend/src/app/app.spec.ts +++ b/frontend/src/app/app.spec.ts @@ -13,11 +13,4 @@ describe('App', () => { const app = fixture.componentInstance; expect(app).toBeTruthy(); }); - - it('should render title', async () => { - const fixture = TestBed.createComponent(App); - await fixture.whenStable(); - const compiled = fixture.nativeElement as HTMLElement; - expect(compiled.querySelector('h1')?.textContent).toContain('Hello, ledmatrix'); - }); }); diff --git a/scripts/dev/run_frontend_dev.sh b/scripts/dev/run_frontend_dev.sh index 8d624aa9b..14cca0560 100755 --- a/scripts/dev/run_frontend_dev.sh +++ b/scripts/dev/run_frontend_dev.sh @@ -14,12 +14,18 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" PROJECT_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" +FASTAPI_PID="" +NG_PID="" + cleanup() { echo "" echo "Stopping servers..." - # Kill all child processes - kill 0 2>/dev/null || true - wait 2>/dev/null || true + for pid in "${FASTAPI_PID:-}" "${NG_PID:-}"; do + if [[ -n "$pid" ]]; then + kill "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + fi + done echo "Done." } trap cleanup EXIT INT TERM diff --git a/src/api/main.py b/src/api/main.py index 9ee9e8547..653af4e83 100644 --- a/src/api/main.py +++ b/src/api/main.py @@ -6,7 +6,6 @@ from pathlib import Path from fastapi import FastAPI -from fastapi.responses import HTMLResponse from fastapi.staticfiles import StaticFiles from starlette.responses import FileResponse, RedirectResponse, Response @@ -91,18 +90,28 @@ def create_app() -> FastAPI: async def root_redirect() -> RedirectResponse: return RedirectResponse(url="/v3", status_code=307) - # Favicon — return 204 No Content (matches current behavior) - @app.get("/favicon.ico", include_in_schema=False) - async def favicon() -> Response: - return Response(status_code=204) - # Mount Angular SPA (when built) — single catch-all serves both # static assets and index.html fallback for client-side routing. # Computed inside create_app() so PROJECT_ROOT can be patched in tests. spa_dist_dir = PROJECT_ROOT / "frontend" / "dist" / "ledmatrix" / "browser" + + # Favicon — serve from SPA dist if present, else 204 No Content + @app.get("/favicon.ico", include_in_schema=False) + async def favicon() -> Response: + spa_favicon = spa_dist_dir / "favicon.ico" + if spa_favicon.is_file(): + return FileResponse(spa_favicon, media_type="image/x-icon") + return Response(status_code=204) + if spa_dist_dir.is_dir(): + # Redirect /v3 → /v3/: the SPA catch-all prevents Starlette's redirect_slashes + # from kicking in for this path, so we add an explicit redirect. + @app.get("/v3", include_in_schema=False) + async def v3_redirect() -> RedirectResponse: + return RedirectResponse(url="/v3/", status_code=307) + _SPA_RESERVED_PREFIXES = ( - "/api/", + "/api", "/docs", "/redoc", "/static", @@ -110,6 +119,7 @@ async def favicon() -> Response: "/openapi.json", "/favicon.ico", ) + _resolved_spa_dist_dir = spa_dist_dir.resolve() @app.get("/{full_path:path}", include_in_schema=False) async def spa_catch_all(full_path: str) -> Response: @@ -120,16 +130,26 @@ async def spa_catch_all(full_path: str) -> Response: return Response(status_code=404) # Serve static file if it exists (JS, CSS, images, etc.) - if full_path and ".." not in full_path: - file_path = spa_dist_dir / full_path - if file_path.is_file(): - media_type = mimetypes.guess_type(str(file_path))[0] - return FileResponse(file_path, media_type=media_type) + # Use path resolution to guard against traversal attacks. + if full_path: + try: + resolved = (_resolved_spa_dist_dir / full_path).resolve() + resolved.relative_to(_resolved_spa_dist_dir) + except ValueError: + return Response(status_code=404) + if resolved.is_file(): + media_type = mimetypes.guess_type(str(resolved))[0] + return FileResponse(resolved, media_type=media_type) + # Return 404 for missing paths with a recognised static-asset MIME type + # (e.g. .js, .css, .png). Paths without a known MIME type (including + # routes like /user/john.doe) fall through to index.html. + if mimetypes.guess_type(full_path)[0] is not None: + return Response(status_code=404) # Fall back to index.html for Angular client-side routing - index_file = spa_dist_dir / "index.html" + index_file = _resolved_spa_dist_dir / "index.html" if index_file.is_file(): - return HTMLResponse(content=index_file.read_text()) + return FileResponse(index_file, media_type="text/html") return Response(status_code=404) return app diff --git a/test/test_api_spa_mount.py b/test/test_api_spa_mount.py index a7695cab0..a33204cb0 100644 --- a/test/test_api_spa_mount.py +++ b/test/test_api_spa_mount.py @@ -60,7 +60,9 @@ def test_spa_catch_all_does_not_intercept_api(self, tmp_path: Path): app = self._create_app_with_spa(tmp_path) client = TestClient(app) - response = client.get("/api/v3/system/health") + response = client.get("/api/v3/system/status") + assert response.status_code == 200 + assert response.headers["content-type"].startswith("application/json") assert "Angular App" not in response.text def test_spa_catch_all_does_not_intercept_docs(self, tmp_path: Path): @@ -75,13 +77,18 @@ def test_spa_catch_all_does_not_intercept_docs(self, tmp_path: Path): assert "Angular App" not in response.text def test_spa_catch_all_does_not_intercept_v3(self, tmp_path: Path): - """HTMX pages at /v3/ must NOT be intercepted by the SPA catch-all.""" + """HTMX pages at /v3/ must NOT be intercepted by the SPA catch-all. + + /v3 (no trailing slash) must redirect to /v3/ — the explicit v3_redirect + route ensures redirect_slashes works even when the SPA catch-all is mounted. + """ from fastapi.testclient import TestClient app = self._create_app_with_spa(tmp_path) client = TestClient(app) - response = client.get("/v3") + response = client.get("/v3", follow_redirects=False) + assert response.status_code == 307 assert "Angular App" not in response.text def test_no_spa_mount_when_dist_missing(self, tmp_path: Path): @@ -96,3 +103,24 @@ def test_no_spa_mount_when_dist_missing(self, tmp_path: Path): # Root should still redirect to /v3 response = client.get("/", follow_redirects=False) assert response.status_code == 307 + + def test_spa_catch_all_does_not_intercept_bare_api(self, tmp_path: Path): + """Bare /api path must NOT be intercepted by the SPA catch-all.""" + from fastapi.testclient import TestClient + + app = self._create_app_with_spa(tmp_path) + client = TestClient(app) + + response = client.get("/api") + assert "Angular App" not in response.text + + def test_spa_missing_asset_returns_404(self, tmp_path: Path): + """Requests for missing files with extensions must return 404, not index.html.""" + from fastapi.testclient import TestClient + + app = self._create_app_with_spa(tmp_path) + client = TestClient(app) + + response = client.get("/missing.js") + assert response.status_code == 404 + assert "Angular App" not in response.text