fix(web): keep exception messages out of API responses (py/stack-trace-exposure)

CHANGELOG.md

@@ -19,6 +19,16 @@ accepts both, but the store flags the old spelling as deprecated
 
 ## Unreleased
 
+- Web API error responses no longer carry an exception's message (CodeQL
+  `py/stack-trace-exposure`). `describe_exception()` now returns a reason
+  code -- the exception type, plus the errno for an `OSError`
+  (`OSError:EIO`, `PermissionError:EACCES`) -- and logs the message
+  instead, so `details` still names the fault without quoting paths, URLs
+  or library internals. The display service status and the on-demand
+  start/stop `service` results keep `active`, `returncode` and `started`
+  but drop systemctl's `stdout`/`stderr`; WiFi, unit-refresh and
+  config-save failures say what failed and point at the log.
+
 ## 3.8.2
 
 The display hands freed memory back to the OS (#774), and sports consolidation

src/web_interface/error_handler.py

@@ -4,6 +4,7 @@
 Provides helpers for consistent error responses across API endpoints.
 """
 
+import errno
 from typing import Any, Optional
 from flask import jsonify
 
@@ -20,31 +21,34 @@
 _MAX_DETAIL_LENGTH = 400
 
 
-def describe_exception(exc: BaseException,
-                       max_length: int = _MAX_DETAIL_LENGTH) -> str:
+def describe_exception(exc: BaseException) -> str:
     """
-    One-line, safe-to-return description of an exception.
+    Machine-readable reason code for an exception, safe to return over HTTP.
 
     The generic "an error occurred; see logs for details" tells a user nothing
     and, when the failure is bad enough, the logs are unreachable too: a device
     whose storage was failing returned that message from every endpoint
     *including* the log viewer, because journalctl could not be executed. The
     underlying `[Errno 5] Input/output error` named the fault immediately.
 
-    Returns "TypeName: message", credentials redacted and length capped. The
-    type alone is worth carrying -- a bare PermissionError says more than any
-    generic sentence.
+    So the type and errno still go back -- "OSError:EIO", "PermissionError:
+    EACCES", "TimeoutExpired" -- but never the exception's message, which can
+    quote paths, URLs, credentials or a library's internals (CodeQL
+    py/stack-trace-exposure). The message is logged here instead, so every
+    reason code a client sees has its full text in the log.
 
     Args:
         exc: The exception to describe
-        max_length: Truncate beyond this many characters
 
     Returns:
-        A single-line description, never empty
+        "TypeName" or "TypeName:ERRNO", never empty
     """
-    message = str(exc).strip()
-    text = f"{type(exc).__name__}: {message}" if message else type(exc).__name__
-    return redact_text(text, max_length)
+    code = type(exc).__name__
+    exc_errno = getattr(exc, 'errno', None)
+    if isinstance(exc_errno, int) and exc_errno in errno.errorcode:
+        code = f"{code}:{errno.errorcode[exc_errno]}"
+    logger.warning("Error reported to the client as %s: %s", code, redact_text(str(exc)))
+    return code
 
 
 def redact_text(text: str, max_length: int = _MAX_DETAIL_LENGTH) -> str:

src/wifi_manager.py

@@ -1369,7 +1369,7 @@ def _fail(msg):
                         self.enable_ap_mode(force=True)
                     except Exception as ap_error:  # nosec B110 - last-resort; do not re-raise, but log for debugging
                         logger.error("Last-resort AP mode enable failed in recovery path: %s", ap_error, exc_info=True)
-            return False, str(e)
+            return False, f"Connection failed ({type(e).__name__}); see logs for details"
 
     def _failsafe_ap(self, enabled_msg: str, failed_msg: str) -> Tuple[bool, str]:
         """Force the setup AP up after a connect that left no working network,
@@ -1585,7 +1585,7 @@ def _connect_nmcli(self, ssid: str, password: str) -> Tuple[bool, str]:
         except Exception as e:
             logger.error(f"Error connecting with nmcli: {e}")
             self._show_led_message("Connection error", duration=5)
-            return False, str(e)
+            return False, f"Connection failed ({type(e).__name__}); see logs for details"
 
     # 802.11 caps an SSID at 32 octets. Control characters cannot appear in a
     # real one, and a leading "-" would be read by nmcli as an option rather
@@ -1725,7 +1725,7 @@ def disconnect_from_network(self, skip_ap_check: bool = False) -> Tuple[bool, st
                 return False, "nmcli is required to disconnect from WiFi"
         except Exception as e:
             logger.error(f"Error disconnecting from WiFi: {e}")
-            return False, str(e)
+            return False, f"Disconnect failed ({type(e).__name__}); see logs for details"
 
     def _ensure_wifi_radio_enabled(self, max_retries: int = 3) -> bool:
         """
@@ -2004,7 +2004,7 @@ def enable_ap_mode(self, force: bool = False) -> Tuple[bool, str]:
             return False, "No WiFi tools available (nmcli, hostapd, or dnsmasq required)"
         except Exception as e:
             logger.error(f"Error in enable_ap_mode: {e}")
-            return False, str(e)
+            return False, f"Could not enable AP mode ({type(e).__name__}); see logs for details"
 
     def _mark_forced(self) -> None:
         """Record that AP mode was forced on, so the periodic check leaves it
@@ -2099,10 +2099,10 @@ def _enable_ap_mode_hostapd(self) -> Tuple[bool, str]:
                 return True, "AP mode enabled"
             except Exception as e:
                 logger.error(f"Error starting AP services: {e}")
-                return False, str(e)
+                return False, f"Could not enable AP mode ({type(e).__name__}); see logs for details"
         except Exception as e:
             logger.error(f"Error enabling AP mode: {e}")
-            return False, str(e)
+            return False, f"Could not enable AP mode ({type(e).__name__}); see logs for details"
 
     def _enable_ap_mode_nmcli_hotspot(self) -> Tuple[bool, str]:
         """
@@ -2227,7 +2227,7 @@ def _enable_ap_mode_nmcli_hotspot(self) -> Tuple[bool, str]:
             logger.error(f"Error starting AP mode with nmcli: {e}")
             self._remove_nm_dnsmasq_captive_conf()
             self._show_led_message("Setup mode error", duration=5)
-            return False, str(e)
+            return False, f"Could not enable AP mode ({type(e).__name__}); see logs for details"
 
     def _get_ap_status_nmcli(self) -> Dict:
         """
@@ -2409,10 +2409,10 @@ def disable_ap_mode(self) -> Tuple[bool, str]:
                 return True, "AP mode disabled"
             except Exception as e:
                 logger.error(f"Error stopping AP services: {e}")
-                return False, str(e)
+                return False, f"Could not disable AP mode ({type(e).__name__}); see logs for details"
         except Exception as e:
             logger.error(f"Error disabling AP mode: {e}")
-            return False, str(e)
+            return False, f"Could not disable AP mode ({type(e).__name__}); see logs for details"
 
     def _create_hostapd_config(self):
         """Create hostapd configuration file"""

test/test_api_v3_display_modes.py

@@ -147,7 +147,8 @@ def test_a_failure_is_reported_the_way_every_other_handler_reports_one(
             side_effect=RuntimeError("disk is gone"))
         resp = api_v3_client.get('/api/v3/display/modes')
         assert resp.status_code == 500
-        assert 'disk is gone' in resp.get_json()['details']
+        assert resp.get_json()['details'] == 'RuntimeError'
+        assert 'disk is gone' not in json.dumps(resp.get_json())
 
     def test_credentials_in_the_exception_are_redacted(self, api_v3_module, api_v3_client):
         """describe_exception is what makes returning detail safe."""

test/test_api_v3_no_exception_text.py

@@ -0,0 +1,147 @@
+"""No API response carries an exception's message (CodeQL py/stack-trace-exposure).
+
+One representative route per file that had open alerts. Each forces a failure
+whose message holds a marker and asserts the marker is nowhere in the body:
+the message goes to the log, the client gets a fixed message plus a reason
+code (describe_exception: the type, and the errno for an OSError).
+"""
+
+import json
+import sys
+from pathlib import Path
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+sys.path.insert(0, str(Path(__file__).parent.parent))
+
+from test._api_v3_test_helpers import api_v3_client, api_v3_module  # noqa: F401,E402
+
+LEAK = "LEAKED-/home/pi/secret token=abc123"
+API = "web_interface.blueprints.api_v3"
+
+
+def _assert_no_leak(response):
+    body = response.get_data(as_text=True)
+    assert "LEAKED" not in body, body
+    assert "abc123" not in body, body
+    return json.loads(body)
+
+
+def test_display_service_status_drops_systemctl_output(api_v3_module, api_v3_client,
+                                                      monkeypatch):
+    """display.py: the on-demand routes return the service status verbatim."""
+    api_v3_module.api_v3.cache_manager.get.return_value = None
+    monkeypatch.setattr(f"{API}.display.display_state.read_state", lambda: None)
+    with patch(f"{API}.subprocess.run", side_effect=OSError(13, LEAK)):
+        body = _assert_no_leak(api_v3_client.get("/api/v3/display/on-demand/status"))
+    assert body["data"]["service"] == {"active": False, "returncode": -1}
+
+
+@pytest.mark.parametrize("helper", ["_ensure_display_service_running",
+                                    "_stop_display_service"])
+def test_service_results_keep_returncode_but_not_output(api_v3_module, helper):
+    """display.py start/stop: returncode/active/started stay, stdout/stderr go."""
+    failed = MagicMock(returncode=1, stdout=LEAK, stderr=LEAK)
+    with patch(f"{API}.subprocess.run", return_value=failed):
+        result = getattr(api_v3_module, helper)()
+    assert "LEAKED" not in json.dumps(result)
+    assert result["returncode"] == 1 and result["active"] is False
+    assert "stdout" not in result and "stderr" not in result
+
+
+def test_wifi_connect_failure(api_v3_client):
+    """wifi.py: a raising connect, and the attempt /wifi/status reports after."""
+    with patch("src.wifi_manager.WiFiManager") as cls:
+        cls.return_value._is_ap_mode_active.return_value = False
+        cls.return_value.connect_to_network.side_effect = RuntimeError(LEAK)
+        body = _assert_no_leak(api_v3_client.post(
+            "/api/v3/wifi/connect", json={"ssid": "HomeNet", "password": "pw"}))
+        assert body["details"] == "RuntimeError"
+        cls.return_value.get_wifi_status.return_value = MagicMock(
+            connected=False, ssid=None, ip_address=None, signal=0, ap_mode_active=False)
+        cls.return_value.config = {}
+        status = _assert_no_leak(api_v3_client.get("/api/v3/wifi/status"))
+    assert status["data"]["last_connect_attempt"]["message"] == (
+        "Failed to connect to network (RuntimeError)")
+
+
+def test_wifi_manager_messages_carry_no_exception_text():
+    """src/wifi_manager.py: its (success, message) is what the wifi routes return."""
+    from src.wifi_manager import WiFiManager
+    manager = WiFiManager.__new__(WiFiManager)  # no __init__: no host access
+    manager.get_wifi_status = MagicMock(side_effect=OSError(5, LEAK))
+    success, message = manager.disconnect_from_network()
+    assert success is False
+    assert "LEAKED" not in message and "OSError" in message
+
+
+def test_system_action_exception(api_v3_client):
+    """system.py: execute_system_action's catch-all."""
+    with patch("subprocess.run", side_effect=OSError(5, LEAK)):
+        body = _assert_no_leak(api_v3_client.post(
+            "/api/v3/system/action", json={"action": "stop_display"}))
+    assert body["details"] == "OSError:EIO"
+
+
+def test_calendar_registration_failure(api_v3_client, tmp_path, monkeypatch):
+    """plugin_calendar.py: the auth script could not be run."""
+    plugin_dir = tmp_path / "calendar"
+    plugin_dir.mkdir()
+    (plugin_dir / "credentials.json").write_text("{}", encoding="utf-8")
+    (plugin_dir / "calendar_registration.py").write_text("", encoding="utf-8")
+    monkeypatch.setattr(f"{API}._calendar_plugin_dir", lambda: plugin_dir)
+    with patch(f"{API}.subprocess.run", side_effect=OSError(13, LEAK)):
+        body = _assert_no_leak(api_v3_client.post(
+            "/api/v3/plugins/calendar/authenticate", json={"code": "x"}))
+    assert "EACCES" in body["message"]
+
+
+def test_health_failure(api_v3_client, monkeypatch):
+    """misc.py: get_health's catch-all."""
+    def boom():
+        raise RuntimeError(LEAK)
+    monkeypatch.setattr(f"{API}.misc._get_display_service_status", boom)
+    body = _assert_no_leak(api_v3_client.get("/api/v3/health"))
+    assert body["details"] == "RuntimeError"
+
+
+def test_config_route_failure(api_v3_module, api_v3_client):
+    """error_handler.py: create_error_response, as config.py's routes use it."""
+    api_v3_module.api_v3.config_manager.load_config.side_effect = RuntimeError(LEAK)
+    body = _assert_no_leak(api_v3_client.get("/api/v3/config/schedule"))
+    assert body["details"] == "RuntimeError"
+
+
+def test_plugin_route_failure(api_v3_module, api_v3_client):
+    """plugins.py: an unhandled error in a plugin route."""
+    api_v3_module.api_v3.plugin_catalog.get_all_plugin_info.side_effect = RuntimeError(LEAK)
+    body = _assert_no_leak(api_v3_client.get("/api/v3/plugins/installed"))
+    assert body["details"] == "RuntimeError"
+
+
+def test_starlark_route_failure(api_v3_client):
+    """starlark.py: one of its catch-alls."""
+    with patch(f"{API}._get_starlark_plugin", side_effect=RuntimeError(LEAK)):
+        body = _assert_no_leak(api_v3_client.get("/api/v3/starlark/status"))
+    assert body["details"] == "RuntimeError"
+
+
+def test_unit_refresh_failure(monkeypatch):
+    """system.py git_pull: perform_core_update appends unit_refresh's message."""
+    from web_interface import unit_refresh
+
+    def boom(*_a, **_k):
+        raise RuntimeError(LEAK)
+    monkeypatch.setattr(unit_refresh, "stale_units", boom)
+    result = unit_refresh.refresh_after_update()
+    assert result["status"] == unit_refresh.FAILED
+    assert "LEAKED" not in result["message"]
+
+
+def test_install_base_requirements_failure(api_v3_client):
+    """system.py: a pip install that could not start, in the action's output."""
+    with patch(f"{API}.system._pip_install_requirements", side_effect=OSError(5, LEAK)):
+        body = _assert_no_leak(api_v3_client.post(
+            "/api/v3/system/action", json={"action": "install_base_requirements"}))
+    assert "Failed: OSError:EIO" in body["output"]

test/test_api_v3_registry_endpoints.py

@@ -95,9 +95,10 @@ def test_failure_body_carries_no_traceback_or_paths(
             RuntimeError("failed at /home/user/LEDMatrix/src/secret.py line 42"))
         body = api_v3_client.post(self.URL, json={}).get_json()
         assert "Traceback" not in str(body)
-        # `details` is describe_exception output: one line, type-named,
-        # credential-redacted. It may quote the message, but never a stack.
-        assert body["details"].startswith("RuntimeError:")
+        # `details` is describe_exception output: the type, never the
+        # message or a stack.
+        assert body["details"] == "RuntimeError"
+        assert "secret.py" not in str(body)
         assert "\n" not in body["details"]