2026-05-31 23:58:26 +09:00
|
|
|
# src/app_helpers.py
|
|
|
|
|
import base64
|
fix(routes): log and cleanly 500 on unreadable HTML page (#4637)
* fix(routes): serve 404 instead of 500 when an HTML page file is missing
_serve_html_with_nonce opened the HTML file with no error handling, and
callers such as /backgrounds and /login pass their paths in with no
existence check, so a missing or unreadable file raised an unhandled
OSError that surfaced as a 500. Wrap the read and raise HTTPException(404)
instead; the normal render path (CSP-nonce substitution) is unchanged.
Fixes #4594
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(routes): distinguish missing page (404) from read failure (500)
The previous fix caught a broad OSError and returned 404 for every
failure, which masks real server-side problems (permission errors, I/O
failures) as "not found" and lets them slip past error alerting. Split
FileNotFoundError (genuine 404) from other OSError, which now logs the
exception and returns a generic 500 — without leaking the OS error
string or file path into the response body.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(routes): treat unreadable bundled HTML page as logged 500, not 404
Per PR #4637 review: every caller of the page-render helper serves a fixed,
server-owned template (index/login/backgrounds), never a client-supplied
path. So a missing or unreadable file is a server fault (broken deployment),
not a client "not found" — a 404 there mislabels a server error and hides a
missing core template from 5xx alerting, contradicting the OSError->500
rationale this PR is built on. Collapse both branches into a single logged,
leak-free 500.
Move the helper to src.app_helpers.serve_html_with_nonce so the behavior can
be unit-tested without importing the whole app (app.py is the slim
orchestrator; the test harness stubs src.database, so importing app in tests
is not viable). Add tests pinning missing/unreadable -> 500 (not 404) and
nonce injection on the happy path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-23 17:12:32 +03:00
|
|
|
import logging
|
|
|
|
|
import os
|
|
|
|
|
|
|
|
|
|
from fastapi import HTTPException
|
|
|
|
|
from fastapi.responses import HTMLResponse
|
|
|
|
|
from starlette.requests import Request
|
|
|
|
|
|
|
|
|
|
logger = logging.getLogger(__name__)
|
2026-05-31 23:58:26 +09:00
|
|
|
|
|
|
|
|
def read_if_exists(path: str) -> str:
|
|
|
|
|
"""Read file if it exists, return empty string otherwise."""
|
|
|
|
|
try:
|
|
|
|
|
with open(path, "r", encoding="utf-8") as f:
|
|
|
|
|
return f.read().strip()
|
|
|
|
|
except Exception:
|
|
|
|
|
return ""
|
|
|
|
|
|
|
|
|
|
def file_to_data_url(path: str, mime: str) -> str:
|
|
|
|
|
"""Convert file to data URL."""
|
|
|
|
|
with open(path, "rb") as f:
|
|
|
|
|
b64 = base64.b64encode(f.read()).decode("ascii")
|
|
|
|
|
return f"data:{mime};base64,{b64}"
|
|
|
|
|
|
|
|
|
|
def abs_join(base_dir: str, rel: str) -> str:
|
|
|
|
|
"""Join paths and return absolute path."""
|
|
|
|
|
return os.path.abspath(os.path.join(base_dir, rel))
|
|
|
|
|
|
fix(routes): log and cleanly 500 on unreadable HTML page (#4637)
* fix(routes): serve 404 instead of 500 when an HTML page file is missing
_serve_html_with_nonce opened the HTML file with no error handling, and
callers such as /backgrounds and /login pass their paths in with no
existence check, so a missing or unreadable file raised an unhandled
OSError that surfaced as a 500. Wrap the read and raise HTTPException(404)
instead; the normal render path (CSP-nonce substitution) is unchanged.
Fixes #4594
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(routes): distinguish missing page (404) from read failure (500)
The previous fix caught a broad OSError and returned 404 for every
failure, which masks real server-side problems (permission errors, I/O
failures) as "not found" and lets them slip past error alerting. Split
FileNotFoundError (genuine 404) from other OSError, which now logs the
exception and returns a generic 500 — without leaking the OS error
string or file path into the response body.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(routes): treat unreadable bundled HTML page as logged 500, not 404
Per PR #4637 review: every caller of the page-render helper serves a fixed,
server-owned template (index/login/backgrounds), never a client-supplied
path. So a missing or unreadable file is a server fault (broken deployment),
not a client "not found" — a 404 there mislabels a server error and hides a
missing core template from 5xx alerting, contradicting the OSError->500
rationale this PR is built on. Collapse both branches into a single logged,
leak-free 500.
Move the helper to src.app_helpers.serve_html_with_nonce so the behavior can
be unit-tested without importing the whole app (app.py is the slim
orchestrator; the test harness stubs src.database, so importing app in tests
is not viable). Add tests pinning missing/unreadable -> 500 (not 404) and
nonce injection on the happy path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-23 17:12:32 +03:00
|
|
|
def serve_html_with_nonce(request: Request, file_path: str) -> HTMLResponse:
|
|
|
|
|
"""Read an app-bundled HTML page and inject the CSP nonce into inline <script> tags.
|
|
|
|
|
|
|
|
|
|
Callers pass fixed, server-owned template paths (index/login/backgrounds),
|
|
|
|
|
never a client-supplied path. So any read failure here — a missing file
|
|
|
|
|
(broken deployment) or a permission/IO error — is a server fault, not a
|
|
|
|
|
client "not found": map all of them to a logged 500 so a missing core
|
|
|
|
|
template surfaces in 5xx alerting instead of hiding behind a 404. If a
|
|
|
|
|
future caller serves a client-influenced path where 404 is correct, branch
|
|
|
|
|
that at the call site rather than defaulting this shared helper to 404.
|
|
|
|
|
"""
|
|
|
|
|
try:
|
|
|
|
|
with open(file_path, "r", encoding="utf-8") as f:
|
|
|
|
|
html = f.read()
|
|
|
|
|
except OSError:
|
|
|
|
|
logger.exception("Failed to read page %s", file_path)
|
|
|
|
|
raise HTTPException(500, "Internal server error")
|
|
|
|
|
nonce = getattr(request.state, "csp_nonce", "")
|
|
|
|
|
html = html.replace("{{CSP_NONCE}}", nonce)
|
|
|
|
|
return HTMLResponse(html)
|
|
|
|
|
|
|
|
|
|
|
2026-05-31 23:58:26 +09:00
|
|
|
def inside_base_dir(base_dir: str, path: str) -> bool:
|
|
|
|
|
"""Check if path is inside base directory."""
|
2026-06-03 01:00:04 +01:00
|
|
|
if not isinstance(base_dir, str) or not isinstance(path, str):
|
|
|
|
|
return False
|
2026-05-31 23:58:26 +09:00
|
|
|
base = os.path.realpath(base_dir)
|
|
|
|
|
p = os.path.realpath(path)
|
|
|
|
|
try:
|
|
|
|
|
return os.path.commonpath([base, p]) == base
|
|
|
|
|
except Exception:
|
|
|
|
|
return False
|