fix: keep exception text out of the queue, release, metadata and debug responses (#1643)

Now that the route moves have landed, this takes care of the code
scanning alerts they brought along. Sixteen places in `queue_routes.py`,
`release_routes.py`, `metadata_routes.py` and `system_routes.py` handed
`str(e)` of an unexpected OSError, RuntimeError, TypeError or sqlite3
error back as the 500 body. It can carry file paths, URLs with tokens or
driver text. Same idea as #1558 and #1562.

Each now returns one fixed sentence ("Could not cancel the download",
"Could not read the queue order", "Metadata search failed" and so on)
and the full error and trace stay in the log. The routes covered are the
cover proxy, cancel, retry, priority, reorder, queue order and active
downloads; release download, search, source list and source record; the
metadata providers, config, search and book routes; and the debug
endpoint.

I left alone the messages that are written for the person using the app:
what a metadata provider raises as a ValueError or RuntimeError
("Hardcover API key is invalid"), `SourceUnavailableError`, and the
reason `queue_release` gives. CodeQL still flags those, as I read it
those are the ones that should be dismissed as intended, but that is
your call. The same pattern is in the settings and onboarding routes,
which aren't flagged, so I didn't touch them.

There are 15 new tests, one per fixed route except the debug endpoint,
which only exists in debug mode. All 15 fail without the change. Two
queue tests that asserted the old text now assert the fixed message. The
full suite, ruff and basedpyright are clean.

@calibrain I pushed through the refactors keeping them the same
functionality, this addresses "issues" flagged in them, I think some of
these were dismissed before, but I gathered them all up here in a PR
that you should have a say on as it is changing long standing code.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

<!-- coderabbit-ai-generated:v1 -->

## Summary by CodeRabbit

* **Bug Fixes**
* Error responses now show generic messages instead of operational
exception details across metadata, queue, release, and debug endpoints.
HTTP status codes and error logging remain unchanged.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

shelfmark/core/metadata_routes.py

@@ -171,7 +171,7 @@ def register_metadata_routes(
             )
         except _IMPORT_OPERATIONAL_ERRORS as e:
             logger.error_trace(f"Metadata providers error: {e}")
-            return jsonify({"error": str(e)}), 500
+            return jsonify({"error": "Could not load the metadata providers"}), 500
 
     @app.route("/api/metadata/config", methods=["GET"])
     @login_required
@@ -239,7 +239,7 @@ def register_metadata_routes(
             )
         except _IMPORT_OPERATIONAL_ERRORS as e:
             logger.error_trace(f"Metadata config error: {e}")
-            return jsonify({"error": str(e)}), 500
+            return jsonify({"error": "Could not load the metadata configuration"}), 500
 
     @app.route("/api/metadata/search", methods=["GET"])
     @login_required
@@ -393,7 +393,7 @@ def register_metadata_routes(
             return jsonify(response_data)
         except _IMPORT_OPERATIONAL_ERRORS as e:
             logger.error_trace(f"Metadata search error: {e}")
-            return jsonify({"error": str(e)}), 500
+            return jsonify({"error": "Metadata search failed"}), 500
 
     @app.route("/api/metadata/field-options", methods=["GET"])
     @login_required
@@ -480,7 +480,7 @@ def register_metadata_routes(
             return jsonify({"error": str(e)}), 503
         except (OSError, TypeError, sqlite3.Error) as e:
             logger.error_trace(f"Metadata book error: {e}")
-            return jsonify({"error": str(e)}), 500
+            return jsonify({"error": "Could not load the book"}), 500
 
     @app.route("/api/metadata/book/<provider>/<book_id>/targets", methods=["GET"])
     @login_required

shelfmark/core/queue_routes.py

@@ -346,7 +346,7 @@ def register_queue_routes(
             response.headers["X-Cache"] = "MISS"
         except _IMPORT_OPERATIONAL_ERRORS as e:
             logger.error_trace(f"Cover fetch error: {e}")
-            return jsonify({"error": str(e)}), 500
+            return jsonify({"error": "Could not fetch the cover image"}), 500
         else:
             return response
 
@@ -398,7 +398,7 @@ def register_queue_routes(
             return jsonify({"error": "Failed to cancel download or book not found"}), 404
         except _OPERATIONAL_ERRORS as e:
             logger.error_trace(f"Cancel download error: {e}")
-            return jsonify({"error": str(e)}), 500
+            return jsonify({"error": "Could not cancel the download"}), 500
 
     @app.route("/api/download/<path:book_id>/retry", methods=["POST"])
     @login_required
@@ -479,7 +479,7 @@ def register_queue_routes(
             return jsonify({"error": error or "Download cannot be retried"}), 409
         except _OPERATIONAL_ERRORS as e:
             logger.error_trace(f"Retry download error: {e}")
-            return jsonify({"error": str(e)}), 500
+            return jsonify({"error": "Could not retry the download"}), 500
 
     @app.route("/api/queue/<path:book_id>/priority", methods=["PUT"])
     @login_required
@@ -531,7 +531,7 @@ def register_queue_routes(
             return jsonify({"error": "Invalid priority value"}), 400
         except _OPERATIONAL_ERRORS as e:
             logger.error_trace(f"Set priority error: {e}")
-            return jsonify({"error": str(e)}), 500
+            return jsonify({"error": "Could not set the priority"}), 500
 
     @app.route("/api/queue/reorder", methods=["POST"])
     @login_required
@@ -587,7 +587,7 @@ def register_queue_routes(
             return jsonify({"error": "Failed to reorder queue"}), 500
         except _OPERATIONAL_ERRORS as e:
             logger.error_trace(f"Reorder queue error: {e}")
-            return jsonify({"error": str(e)}), 500
+            return jsonify({"error": "Could not reorder the queue"}), 500
 
     @app.route("/api/queue/order", methods=["GET"])
     @login_required
@@ -617,7 +617,7 @@ def register_queue_routes(
             return jsonify({"queue": queue_order})
         except _OPERATIONAL_ERRORS as e:
             logger.error_trace(f"Queue order error: {e}")
-            return jsonify({"error": str(e)}), 500
+            return jsonify({"error": "Could not read the queue order"}), 500
 
     @app.route("/api/downloads/active", methods=["GET"])
     @login_required
@@ -647,4 +647,4 @@ def register_queue_routes(
             return jsonify({"active_downloads": active_downloads})
         except _OPERATIONAL_ERRORS as e:
             logger.error_trace(f"Active downloads error: {e}")
-            return jsonify({"error": str(e)}), 500
+            return jsonify({"error": "Could not read the active downloads"}), 500

shelfmark/core/release_routes.py

@@ -459,7 +459,7 @@ def register_release_routes(
             return jsonify({"error": error_msg or "Failed to queue release"}), 500
         except _OPERATIONAL_ERRORS as e:
             logger.error_trace(f"Release download error: {e}")
-            return jsonify({"error": str(e)}), 500
+            return jsonify({"error": "Could not queue the release"}), 500
 
     @app.route("/api/releases", methods=["GET"])
     @login_required
@@ -756,7 +756,7 @@ def register_release_routes(
             return jsonify({"error": str(e)}), 503
         except _IMPORT_OPERATIONAL_ERRORS as e:
             logger.error_trace(f"Releases search error: {e}")
-            return jsonify({"error": str(e)}), 500
+            return jsonify({"error": "Could not search for releases"}), 500
 
     @app.route("/api/release-sources", methods=["GET"])
     @login_required
@@ -774,7 +774,7 @@ def register_release_routes(
             return jsonify(sources)
         except _IMPORT_OPERATIONAL_ERRORS as e:
             logger.error_trace(f"Release sources error: {e}")
-            return jsonify({"error": str(e)}), 500
+            return jsonify({"error": "Could not list the release sources"}), 500
 
     @app.route("/api/release-sources/<source_name>/records/<path:record_id>", methods=["GET"])
     @login_required
@@ -800,4 +800,4 @@ def register_release_routes(
             return jsonify({"error": str(e)}), 503
         except _OPERATIONAL_ERRORS as e:
             logger.error_trace(f"Release source record error: {e}")
-            return jsonify({"error": str(e)}), 500
+            return jsonify({"error": "Could not resolve the record"}), 500

shelfmark/core/system_routes.py

@@ -305,7 +305,7 @@ def register_system_routes(
                 return jsonify({"error": f"Debug script failed: {e.stderr}"}), 500
             except _OPERATIONAL_ERRORS as e:
                 logger.error_trace(f"Debug endpoint error: {e}")
-                return jsonify({"error": str(e)}), 500
+                return jsonify({"error": "Could not create the debug file"}), 500
 
         @app.route("/api/restart", methods=["GET"])
         @admin_required

tests/core/test_error_text_in_responses.py

@@ -0,0 +1,172 @@
+"""An unexpected failure in a route returns a fixed message, never the exception text.
+
+The exception can carry file paths, URLs with tokens or driver messages. The full error
+stays in the log with its trace; the caller gets one sentence per route. Messages that
+were written for the user (a provider's "API key is invalid", a source being down) are
+a different thing and are covered by the tests of those routes.
+"""
+
+from __future__ import annotations
+
+import base64
+import importlib
+from unittest.mock import patch
+
+import pytest
+
+_LEAK = "/srv/shelfmark/secret/path token=abc123"
+
+
+@pytest.fixture(scope="module")
+def main_module():
+    """Import `shelfmark.main` with background startup disabled."""
+    with patch("shelfmark.download.orchestrator.start"):
+        import shelfmark.main as main
+
+        importlib.reload(main)
+        return main
+
+
+@pytest.fixture
+def client(main_module):
+    return main_module.app.test_client()
+
+
+def _boom(*_args, **_kwargs):
+    raise OSError(_LEAK)
+
+
+def _admin_session(client) -> None:
+    with client.session_transaction() as sess:
+        sess["user_id"] = "alice"
+        sess["is_admin"] = True
+        sess["db_user_id"] = 7
+
+
+class _Source:
+    def get_record(self, record_id):
+        raise OSError(_LEAK)
+
+
+def _cover_url() -> str:
+    return "/api/covers/c1?url=" + base64.urlsafe_b64encode(b"https://example.com/c.jpg").decode()
+
+
+def _patches(main_module, name: str):
+    """The patches that make route `name` fail with an OSError carrying _LEAK."""
+    backend = main_module.backend
+    table = {
+        "cancel": [patch.object(backend.book_queue, "get_task", side_effect=_boom)],
+        "retry": [patch.object(backend.book_queue, "get_task", side_effect=_boom)],
+        "priority": [patch.object(backend.book_queue, "get_task", side_effect=_boom)],
+        "reorder": [patch.object(backend, "reorder_queue", side_effect=_boom)],
+        "order": [patch.object(backend, "get_queue_order", side_effect=_boom)],
+        "active": [patch.object(backend, "get_active_downloads", side_effect=_boom)],
+        "cover": [
+            patch("shelfmark.config.env.is_covers_cache_enabled", return_value=True),
+            patch("shelfmark.core.image_cache.get_image_cache", side_effect=_boom),
+        ],
+        "release_download": [patch.object(backend, "queue_release", side_effect=_boom)],
+        "release_search": [
+            patch("shelfmark.metadata_providers.is_provider_registered", return_value=True),
+            patch("shelfmark.metadata_providers.get_provider_kwargs", side_effect=_boom),
+            patch(
+                "shelfmark.release_sources.list_available_sources",
+                return_value=[{"name": "direct_download", "enabled": True}],
+            ),
+            patch("shelfmark.release_sources.source_results_are_releases", return_value=False),
+        ],
+        "release_sources": [
+            patch("shelfmark.release_sources.list_available_sources", side_effect=_boom)
+        ],
+        "release_record": [
+            patch("shelfmark.release_sources.get_source", return_value=_Source()),
+        ],
+        "metadata_providers": [patch("shelfmark.core.metadata_routes.app_config.refresh", _boom)],
+        "metadata_config": [patch("shelfmark.core.metadata_routes.app_config.refresh", _boom)],
+        "metadata_search": [
+            patch("shelfmark.metadata_providers.get_configured_provider", side_effect=_boom)
+        ],
+        "metadata_book": [
+            patch("shelfmark.metadata_providers.is_provider_registered", return_value=True),
+            patch("shelfmark.metadata_providers.get_provider_kwargs", side_effect=_boom),
+        ],
+    }
+    return table[name]
+
+
+CASES = [
+    ("cancel", "delete", "/api/download/t1/cancel", None, "Could not cancel the download"),
+    ("retry", "post", "/api/download/t1/retry", {}, "Could not retry the download"),
+    ("priority", "put", "/api/queue/t1/priority", {"priority": 1}, "Could not set the priority"),
+    (
+        "reorder",
+        "post",
+        "/api/queue/reorder",
+        {"book_priorities": {"a": 0}},
+        "Could not reorder the queue",
+    ),
+    ("order", "get", "/api/queue/order", None, "Could not read the queue order"),
+    ("active", "get", "/api/downloads/active", None, "Could not read the active downloads"),
+    ("cover", "get", _cover_url(), None, "Could not fetch the cover image"),
+    (
+        "release_download",
+        "post",
+        "/api/releases/download",
+        {"source": "direct_download", "source_id": "a", "title": "Dune"},
+        "Could not queue the release",
+    ),
+    (
+        "release_search",
+        "get",
+        "/api/releases?provider=hardcover&book_id=1",
+        None,
+        "Could not search for releases",
+    ),
+    ("release_sources", "get", "/api/release-sources", None, "Could not list the release sources"),
+    (
+        "release_record",
+        "get",
+        "/api/release-sources/direct_download/records/r1",
+        None,
+        "Could not resolve the record",
+    ),
+    (
+        "metadata_providers",
+        "get",
+        "/api/metadata/providers",
+        None,
+        "Could not load the metadata providers",
+    ),
+    (
+        "metadata_config",
+        "get",
+        "/api/metadata/config",
+        None,
+        "Could not load the metadata configuration",
+    ),
+    ("metadata_search", "get", "/api/metadata/search?query=dune", None, "Metadata search failed"),
+    ("metadata_book", "get", "/api/metadata/book/hardcover/1", None, "Could not load the book"),
+]
+
+
+@pytest.mark.parametrize(
+    ("name", "method", "path", "body", "message"), CASES, ids=[c[0] for c in CASES]
+)
+def test_failure_returns_a_fixed_message(main_module, client, name, method, path, body, message):
+    _admin_session(client)
+    patches = _patches(main_module, name)
+    kwargs = {"json": body} if body is not None else {}
+
+    with patch.object(main_module, "get_auth_mode", return_value="builtin"):
+        for p in patches:
+            p.start()
+        try:
+            resp = getattr(client, method)(path, **kwargs)
+        finally:
+            for p in reversed(patches):
+                p.stop()
+
+    assert resp.status_code == 500
+    assert resp.get_json() == {"error": message}
+    assert _LEAK not in resp.get_data(as_text=True)

tests/core/test_queue_routes.py

@@ -258,7 +258,7 @@ class TestCancel:
             "code": "user_identity_unavailable",
         }
 
-    def test_a_failure_in_the_backend_is_a_500_with_its_message(self, main_module, client):
+    def test_a_failure_in_the_backend_is_a_500_with_a_fixed_message(self, main_module, client):
         _login(client, is_admin=True)
         with _builtin(main_module):
             with patch.object(
@@ -267,7 +267,7 @@ class TestCancel:
                 resp = client.delete("/api/download/task-1/cancel")
 
         assert resp.status_code == 500
-        assert resp.get_json() == {"error": "queue broke"}
+        assert resp.get_json() == {"error": "Could not cancel the download"}
 
     def test_the_opaque_id_a_viewer_sees_cancels_the_real_task(self, main_module, client):
         user = _create_user(main_module, prefix="reader")
@@ -440,7 +440,7 @@ class TestQueueViews:
         assert order.status_code == active.status_code == 403
         assert order.get_json()["code"] == active.get_json()["code"] == "user_identity_unavailable"
 
-    def test_a_failure_is_a_500_with_its_message(self, main_module, client):
+    def test_a_failure_is_a_500_with_a_fixed_message(self, main_module, client):
         _login(client, is_admin=True)
         with _builtin(main_module):
             with patch.object(
@@ -449,7 +449,7 @@ class TestQueueViews:
                 resp = client.get("/api/queue/order")
 
         assert resp.status_code == 500
-        assert resp.get_json() == {"error": "order broke"}
+        assert resp.get_json() == {"error": "Could not read the queue order"}
 
 
 class TestStatus: