From 16494a283f684a30a49519b107ff782ec3836fd1 Mon Sep 17 00:00:00 2001 From: hermes Date: Mon, 9 Nov 2026 11:49:41 +1000 Subject: [PATCH] fix: log index failures, and tidy the CSP and the API error response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review findings. - `_ensure_indexes` swallowed a `PyMongoError` with a bare `pass`. An index failure does not stop the app serving, but it means the lookup is a collection scan, which gets slow as the data grows — so it is now logged as a warning with the traceback. - The Content-Security-Policy string is now a module-level `CSP` constant rather than inline in the response handler, and it gained `object-src 'none'` (the app uses no plugins). The test now also asserts that neither `unsafe-inline` nor `unsafe-eval` appears — either would defeat the point of the policy. - `api_run` returned a bare dict for its 404 while the success path used `jsonify`. It is now `jsonify({"error": "not found"})` for consistency. --- src/flask/app_factory.py | 27 +++++++++++++++++++++------ src/flask/mongo/repository.py | 10 ++++++++-- src/flask/views/brews.py | 2 +- tests/test_app.py | 4 ++++ 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/flask/app_factory.py b/src/flask/app_factory.py index f47dc18..5655d8e 100644 --- a/src/flask/app_factory.py +++ b/src/flask/app_factory.py @@ -16,6 +16,26 @@ from flask_wtf import CSRFProtect __all__ = ["create_app"] +#: The Content-Security-Policy the app sends with every response. +#: +#: The app serves only its own vendored assets — CSS, JavaScript, fonts and +#: the logo are all in ``static/`` — so a strict ``'self'`` policy is +#: enforceable. There is no ``'unsafe-inline'`` anywhere, which is why the +#: templates carry no inline scripts or style attributes. +CSP = ( + "default-src 'self'; " + "script-src 'self'; " + "style-src 'self'; " + "img-src 'self' data:; " + "font-src 'self'; " + "connect-src 'self'; " + "form-action 'self'; " + "object-src 'none'; " + "frame-ancestors 'none'; " + "base-uri 'self'" +) + + class ImproperlyConfigured(RuntimeError): """Raised when the app is missing configuration it cannot safely default. @@ -121,12 +141,7 @@ def create_app(config: dict | None = None) -> Flask: response.headers.setdefault("X-Content-Type-Options", "nosniff") response.headers.setdefault("X-Frame-Options", "DENY") response.headers.setdefault("Referrer-Policy", "same-origin") - response.headers.setdefault( - "Content-Security-Policy", - "default-src 'self'; script-src 'self'; style-src 'self'; " - "img-src 'self' data:; font-src 'self'; connect-src 'self'; " - "form-action 'self'; frame-ancestors 'none'; base-uri 'self'", - ) + response.headers.setdefault("Content-Security-Policy", CSP) return response @app.errorhandler(404) diff --git a/src/flask/mongo/repository.py b/src/flask/mongo/repository.py index d2b7d28..7f74d7f 100644 --- a/src/flask/mongo/repository.py +++ b/src/flask/mongo/repository.py @@ -59,8 +59,14 @@ class BeerRepository: [("beer_run_id", 1), ("date", 1)], name="run_date_1" ) except PyMongoError: - # An index failure must not stop the app from serving. - pass + # An index failure must not stop the app from serving, but it + # should be visible: without the index the lookup is a collection + # scan, which gets slow as the data grows. + logger.warning( + "Could not create the Mongo indexes; the lookups will be " + "collection scans until they exist", + exc_info=True, + ) # -- users -------------------------------------------------------------- # diff --git a/src/flask/views/brews.py b/src/flask/views/brews.py index 399bc53..3998241 100644 --- a/src/flask/views/brews.py +++ b/src/flask/views/brews.py @@ -196,7 +196,7 @@ def api_run(run_id: str): """JSON detail of a single run.""" run = repo().run(run_id) if run is None: - return {"error": "not found"}, 404 + return jsonify({"error": "not found"}), 404 meta = repo().run_metadata(run_id) summary = run_summary(run, og_override=meta.get("og_points")) diff --git a/tests/test_app.py b/tests/test_app.py index fed1f4a..b33f4ba 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -491,9 +491,13 @@ def test_security_headers_are_set(logged_in): assert csp is not None assert "default-src 'self'" in csp assert "script-src 'self'" in csp + assert "object-src 'none'" in csp assert "frame-ancestors 'none'" in csp # No CDN is used, so nothing external should be allowed. assert "https://" not in csp + # 'unsafe-inline' would defeat the point of the policy. + assert "unsafe-inline" not in csp + assert "unsafe-eval" not in csp def test_security_headers_are_set_on_the_login_page(client):