diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ef00eff..d4620e98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,14 @@ accepts both, but the store flags the old spelling as deprecated `a/b` or `..` resolve to nothing everywhere. Installs where each directory is named for its manifest id, the installer's layout, behave as before. +- `/api/v3` routes answer an exception they don't handle themselves from one + blueprint error handler, with the same `{status, message, details}` body the + 53 removed per-route catch-alls returned. `ErrorCategory` and the + `error_category` key are removed from `src.web_interface.errors` (nothing read + them); `exception_error_response()` replaces the `from_exception` + + `error_response` pairs. A failing plugin action script's error now names the + real failure instead of `UnboundLocalError`. + - `FontManager.get_font()` returns a BDF font at its native size when asked for a size the file doesn't contain (5x7.bdf at 8 or 10px, say). It used to return PIL's default font, a different typeface, so a plugin that relied on diff --git a/docs/REST_API_REFERENCE.md b/docs/REST_API_REFERENCE.md index 8406403e..2e22a30f 100644 --- a/docs/REST_API_REFERENCE.md +++ b/docs/REST_API_REFERENCE.md @@ -2111,13 +2111,18 @@ Errors use one of two shapes. Most endpoints answer: } ``` -Endpoints built on the structured error helper add a code and category: +An exception no route anticipated gets this shape too, with a 500, the +message `An error occurred; see logs for details`, and `details` naming the +exception type and text (credentials redacted). The api_v3 blueprint's +error handler produces it, so it is the same for every `/api/v3` route. + +Endpoints built on the structured error helper add a code, and usually +suggested fixes (the web UI's error dialog lists them): ```json { "status": "error", "error_code": "CONFIG_SAVE_FAILED", - "error_category": "configuration", "message": "Error description", "details": "optional", "context": { }, diff --git a/src/web_interface/api_helpers.py b/src/web_interface/api_helpers.py index fe6471e2..dc92b249 100644 --- a/src/web_interface/api_helpers.py +++ b/src/web_interface/api_helpers.py @@ -9,7 +9,7 @@ from typing import Any, Optional, Dict, Tuple from flask import jsonify, request from src.web_interface.error_handler import create_error_response, create_success_response -from src.web_interface.errors import ErrorCode +from src.web_interface.errors import ErrorCode, WebInterfaceError def success_response( @@ -75,6 +75,40 @@ def error_response( ) +def exception_error_response( + exc: Exception, + error_code: ErrorCode, + *, + with_context: bool = True, + status_code: int = 500 +): + """ + error_response() for a caught exception, built by WebInterfaceError. + + The message is the code's fixed, user-facing one -- never the exception + text. `details` comes from the exception's own `context` dict when it has + one, and `context` records the exception type. with_context=False leaves + the context out, as the operation-history routes always have. + + Args: + exc: The exception being reported + error_code: Error code + with_context: Whether to include the context (exception type) + status_code: HTTP status code + + Returns: + Flask jsonify response with status code + """ + error = WebInterfaceError.from_exception(exc, error_code) + return error_response( + error.error_code, + error.message, + details=error.details, + context=error.context if with_context else None, + status_code=status_code + ) + + def validate_request_json(required_fields: list, data: Optional[Dict] = None) -> Tuple[Optional[Dict], Optional[Any]]: """ Validate request JSON has required fields. diff --git a/src/web_interface/error_handler.py b/src/web_interface/error_handler.py index 6554568c..376636a2 100644 --- a/src/web_interface/error_handler.py +++ b/src/web_interface/error_handler.py @@ -7,9 +7,7 @@ Provides helpers for consistent error responses across API endpoints. from typing import Any, Optional from flask import jsonify -from src.web_interface.errors import ( - WebInterfaceError, ErrorCode, ErrorCategory -) +from src.web_interface.errors import WebInterfaceError, ErrorCode from src.logging_config import get_logger from src.redaction import redact_credentials @@ -72,6 +70,39 @@ def redact_text(text: str, max_length: int = _MAX_DETAIL_LENGTH) -> str: return text +# What a failure nothing anticipated says. The detail beside it carries the +# actual diagnosis; this sentence only points at where the traceback went. +UNHANDLED_ERROR_MESSAGE = 'An error occurred; see logs for details' + + +def unhandled_exception_payload(exc: BaseException) -> dict: + """JSON body for an exception no route handled: status, message, details. + + Deliberately no `error_code`. The plugin API client (api_client.js) passes + a body that has one straight to the rich error modal, and wraps one that + has none as a plain API_ERROR toast; the api_v3 routes answered this shape + from their own catch-alls for years, so the UI is built around it. + """ + return { + 'status': 'error', + 'message': UNHANDLED_ERROR_MESSAGE, + 'details': describe_exception(exc), + } + + +def http_exception_payload(error) -> dict: + """JSON body for a werkzeug HTTPException (405, 400, 415, 413...). + + Same shape web_interface/app.py's global handler returns, so a 4xx raised + inside an api_v3 route reads the same as one raised anywhere else. + """ + return { + 'status': 'error', + 'error_code': (error.name or 'HTTP_ERROR').upper().replace(' ', '_'), + 'message': error.description, + } + + def create_error_response( error_code: ErrorCode, message: str, diff --git a/src/web_interface/errors.py b/src/web_interface/errors.py index bb7c6e07..2fe03995 100644 --- a/src/web_interface/errors.py +++ b/src/web_interface/errors.py @@ -1,7 +1,7 @@ """ Structured error handling for web interface. -Provides error codes, categories, and consistent error response formatting. +Provides error codes and consistent error response formatting. """ from enum import Enum @@ -9,17 +9,6 @@ from typing import Dict, Any, Optional, List from dataclasses import dataclass -class ErrorCategory(Enum): - """Error categories for classification.""" - CONFIGURATION = "configuration" - PLUGIN = "plugin" - VALIDATION = "validation" - NETWORK = "network" - PERMISSION = "permission" - SYSTEM = "system" - UNKNOWN = "unknown" - - class ErrorCode(Enum): """Error codes for specific error types.""" # Configuration errors @@ -63,12 +52,11 @@ class WebInterfaceError: """ Structured error for web interface responses. - Provides consistent error format with error codes, categories, - messages, and context. + Provides consistent error format with error codes, messages, and + context. """ error_code: ErrorCode message: str - category: ErrorCategory details: Optional[str] = None context: Optional[Dict[str, Any]] = None suggested_fixes: Optional[List[str]] = None @@ -78,7 +66,6 @@ class WebInterfaceError: self, error_code: ErrorCode, message: str, - category: Optional[ErrorCategory] = None, details: Optional[str] = None, context: Optional[Dict[str, Any]] = None, suggested_fixes: Optional[List[str]] = None, @@ -86,7 +73,6 @@ class WebInterfaceError: ): self.error_code = error_code self.message = message - self.category = category or self._infer_category(error_code) self.details = details self.context = context or {} # `is None`, not truthiness: an explicit [] means "this caller has @@ -96,25 +82,6 @@ class WebInterfaceError: else self._get_default_suggestions(error_code)) self.original_error = original_error - def _infer_category(self, error_code: ErrorCode) -> ErrorCategory: - """Infer error category from error code.""" - code_str = error_code.value - - if code_str.startswith("CONFIG_"): - return ErrorCategory.CONFIGURATION - elif code_str.startswith("PLUGIN_"): - return ErrorCategory.PLUGIN - elif code_str.startswith("VALIDATION_") or code_str.startswith("SCHEMA_") or code_str == "INVALID_INPUT": - return ErrorCategory.VALIDATION - elif code_str.startswith("NETWORK_") or code_str == "API_ERROR" or code_str == "TIMEOUT": - return ErrorCategory.NETWORK - elif code_str.startswith("PERMISSION_") or code_str == "FILE_PERMISSION_ERROR": - return ErrorCategory.PERMISSION - elif code_str.startswith("SYSTEM_") or code_str == "SERVICE_UNAVAILABLE": - return ErrorCategory.SYSTEM - else: - return ErrorCategory.UNKNOWN - def _get_default_suggestions(self, error_code: ErrorCode) -> List[str]: """Get default suggested fixes for error code.""" suggestions_map = { @@ -178,7 +145,6 @@ class WebInterfaceError: result = { "status": "error", "error_code": self.error_code.value, - "error_category": self.category.value, "message": self.message, } @@ -197,7 +163,7 @@ class WebInterfaceError: def from_exception( cls, exception: Exception, - error_code: Optional[ErrorCode] = None, + error_code: ErrorCode, context: Optional[Dict[str, Any]] = None ) -> 'WebInterfaceError': """ @@ -205,13 +171,9 @@ class WebInterfaceError: Args: exception: Exception to convert - error_code: Optional specific error code + error_code: The error code to report context: Optional additional context """ - # Infer error code from exception type if not provided - if not error_code: - error_code = cls._infer_error_code(exception) - # Build context error_context = context or {} error_context['exception_type'] = type(exception).__name__ @@ -252,26 +214,6 @@ class WebInterfaceError: } return messages.get(error_code, "An unexpected error occurred") - @classmethod - def _infer_error_code(cls, exception: Exception) -> ErrorCode: - """Infer error code from exception type.""" - exception_name = type(exception).__name__ - - if "Config" in exception_name: - return ErrorCode.CONFIG_LOAD_FAILED - elif "Plugin" in exception_name: - return ErrorCode.PLUGIN_LOAD_FAILED - elif "Permission" in exception_name or "Access" in exception_name: - return ErrorCode.PERMISSION_DENIED - elif "Validation" in exception_name or "Schema" in exception_name: - return ErrorCode.VALIDATION_ERROR - elif "Network" in exception_name or "Connection" in exception_name: - return ErrorCode.NETWORK_ERROR - elif "Timeout" in exception_name: - return ErrorCode.TIMEOUT - else: - return ErrorCode.UNKNOWN_ERROR - @classmethod def _get_exception_details(cls, exception: Exception) -> Optional[str]: """Get additional details from exception.""" diff --git a/test/test_web_error_detail.py b/test/test_web_error_detail.py index 6324bc01..2fb3a2f7 100644 --- a/test/test_web_error_detail.py +++ b/test/test_web_error_detail.py @@ -230,6 +230,52 @@ class TestHandlersCarryDetail: "handlers returning the generic message without %s: %r" % ("both a traceback log and the detail", offenders)) + def test_no_api_v3_route_copies_the_blueprint_handler(self): + """The generic catch-all lives once, on the blueprint. + + Fifty-three routes carried their own copy of it -- log with exc_info, + return {status, "An error occurred; see logs for details", + describe_exception(e)}, 500 -- until they were folded into + `_api_v3_unhandled_exception`. A new copy changes nothing a caller + sees, so nothing else would notice it; this does. A handler that says + something *different* (its own message, extra keys, cleanup) is fine. + """ + import ast + import pathlib + + generic = "An error occurred; see logs for details" + copies = [] + for path in sorted(pathlib.Path("web_interface/blueprints/api_v3").glob("*.py")): + tree = ast.parse(path.read_text(encoding="utf-8")) + for fn in [n for n in ast.walk(tree) if isinstance(n, ast.FunctionDef)]: + for h in ast.walk(fn): + if not (isinstance(h, ast.ExceptHandler) + and isinstance(h.type, ast.Name) + and h.type.id == "Exception"): + continue + for r in [n for n in h.body if isinstance(n, ast.Return)]: + v = r.value + if not (isinstance(v, ast.Tuple) and len(v.elts) == 2 + and isinstance(v.elts[0], ast.Call) + and getattr(v.elts[0].func, "id", None) == "jsonify" + and v.elts[0].args + and isinstance(v.elts[0].args[0], ast.Dict)): + continue + d = v.elts[0].args[0] + keys = {k.value for k in d.keys if isinstance(k, ast.Constant)} + message = [val.value for k, val in zip(d.keys, d.values) + if isinstance(k, ast.Constant) and k.value == "message" + and isinstance(val, ast.Constant)] + if keys == {"status", "message", "details"} and message == [generic]: + copies.append((path.name, fn.name)) + + # One is not a copy: execute_plugin_action's step-1 handler sits + # inside the route's `except subprocess.TimeoutExpired` arm, which + # would turn a plugin's own timeout into a 408 if this let it through. + assert copies == [("plugins.py", "execute_plugin_action")], ( + "these handlers duplicate the api_v3 blueprint's errorhandler; " + "delete them and let the exception propagate: %r" % copies) + def test_client_errors_keep_their_own_status(self): """A 405 must not be reported as a server-side UNKNOWN_ERROR. diff --git a/test/web_interface/test_api_v3_unhandled_errors.py b/test/web_interface/test_api_v3_unhandled_errors.py new file mode 100644 index 00000000..5bd14150 --- /dev/null +++ b/test/web_interface/test_api_v3_unhandled_errors.py @@ -0,0 +1,221 @@ +"""An exception no api_v3 route catches is answered once, by the blueprint. + +Fifty-three routes used to end in a copy of the same catch-all: + + except Exception as e: + logger.error(..., exc_info=True) + return jsonify({'status': 'error', + 'message': 'An error occurred; see logs for details', + 'details': describe_exception(e)}), 500 + +They were removed in favour of one errorhandler on the api_v3 blueprint. These +tests pin that the answer did not change: the same status, exactly the same +keys and values, credentials still redacted, and the traceback still logged. + +The exact-equality matters. web_interface/app.py's global handler answers with +an extra `error_code: UNKNOWN_ERROR`, and the plugin API client routes a body +that carries an error_code to a different UI path (api_client.js) -- so +"falls through to the global handler" would not have been the same answer. +""" + +import logging + +import pytest +from flask import Flask +from werkzeug.exceptions import UnsupportedMediaType + +from src.web_interface.error_handler import describe_exception +from web_interface.blueprints.api_v3 import api_v3 + +# A credential in three of the forms describe_exception redacts. +FORCED = RuntimeError( + "forced failure token=SECRET123 at https://u:pw1@example.com/x?api_key=K1") + +# What every removed catch-all returned, written out rather than imported so +# a change to the shared payload cannot also change the expectation. +EXPECTED = { + 'status': 'error', + 'message': 'An error occurred; see logs for details', + 'details': describe_exception(FORCED), +} + +MANAGERS = ("config_manager", "plugin_manager", "plugin_store_manager", + "saved_repositories_manager", "schema_manager", "operation_queue", + "plugin_state_manager", "operation_history", "cache_manager") + + +class Boom: + """A manager that fails on any use -- attribute, truthiness, call.""" + + def _raise(self, *args, **kwargs): + raise FORCED + + __getattr__ = _raise + __bool__ = _raise + __call__ = _raise + __iter__ = _raise + __len__ = _raise + + +@pytest.fixture +def exploding_managers(monkeypatch): + for name in MANAGERS: + monkeypatch.setattr(api_v3, name, Boom(), raising=False) + # The WiFi routes build their own manager rather than using one above. + import src.wifi_manager + monkeypatch.setattr(src.wifi_manager, "WiFiManager", Boom()) + + +@pytest.fixture +def client(exploding_managers): + """The blueprint alone, on an app with no error handlers of its own.""" + app = Flask(__name__) + app.register_blueprint(api_v3, url_prefix="/api/v3") + return app.test_client() + + +# A sample of routes whose catch-all was removed, across every module that +# lost one. Each reaches a manager (or WiFiManager) inside what used to be +# the try block. +REMOVED_CATCH_ALLS = [ + ("GET", "/api/v3/config/main", None), + ("GET", "/api/v3/config/secrets", None), + ("GET", "/api/v3/display/modes", None), + ("POST", "/api/v3/display/on-demand/stop", {}), + ("GET", "/api/v3/cache/list", None), + ("GET", "/api/v3/plugins/installed", None), + ("GET", "/api/v3/plugins/health", None), + ("GET", "/api/v3/plugins/metrics/some-plugin", None), + ("GET", "/api/v3/plugins/schema?plugin_id=some-plugin", None), + ("GET", "/api/v3/plugins/store/list", None), + ("GET", "/api/v3/plugins/saved-repositories", None), + ("POST", "/api/v3/plugins/install", {"plugin_id": "some-plugin"}), + ("POST", "/api/v3/plugins/config/reset?plugin_id=some-plugin", {}), + ("GET", "/api/v3/plugins/limits/some-plugin", None), + ("GET", "/api/v3/wifi/status", None), + ("POST", "/api/v3/wifi/disconnect", {}), +] + + +@pytest.mark.parametrize("method,url,body", REMOVED_CATCH_ALLS, + ids=[f"{m} {u}" for m, u, _ in REMOVED_CATCH_ALLS]) +def test_the_answer_is_what_the_catch_all_returned(client, caplog, method, url, body): + with caplog.at_level(logging.ERROR, logger="web_interface.blueprints.api_v3"): + resp = client.open(url, method=method, json=body) + + assert resp.status_code == 500 + assert resp.get_json() == EXPECTED + + # The promise in the message: the traceback is in the log. + records = [r for r in caplog.records + if r.name == "web_interface.blueprints.api_v3" + and r.levelno >= logging.ERROR and r.exc_info] + assert records, "the unhandled exception was not logged with its traceback" + assert records[-1].exc_info[1] is FORCED + + +def test_credentials_are_redacted_from_the_detail(client): + body = client.get("/api/v3/plugins/installed").get_json() + for secret in ("SECRET123", "pw1", "K1"): + assert secret not in body["details"] + assert "" in body["details"] + assert body["details"].startswith("RuntimeError: forced failure") + + +def test_a_client_error_keeps_its_own_status(client): + """HTTPExceptions subclass Exception; a 415 must not become a 500.""" + resp = client.post("/api/v3/plugins/assets/delete", data="not json", + content_type="text/plain") + assert resp.status_code == 415 + body = resp.get_json() + assert body == { + 'status': 'error', + 'error_code': 'UNSUPPORTED_MEDIA_TYPE', + 'message': body['message'], + } + assert "Content-Type" in body['message'] + + +class TestInTheRealApp: + """Mounted in web_interface/app.py, beside its global handlers.""" + + @pytest.fixture + def web_app(self): + import web_interface.app as web_app + return web_app + + def test_the_blueprint_handler_answers_not_the_global_one( + self, web_app, exploding_managers): + resp = web_app.app.test_client().get("/api/v3/plugins/installed") + assert resp.status_code == 500 + assert resp.get_json() == EXPECTED + + def test_client_errors_read_the_same_as_the_global_handler( + self, web_app, exploding_managers): + """The blueprint's 4xx shape must not drift from app.py's.""" + resp = web_app.app.test_client().post( + "/api/v3/plugins/assets/delete", data="not json", + content_type="text/plain") + with web_app.app.test_request_context(): + global_resp, global_status = web_app.handle_exception( + UnsupportedMediaType(description=resp.get_json()['message'])) + assert resp.status_code == global_status == 415 + assert resp.get_json() == global_resp.get_json() + + def test_global_handler_shape(self, web_app): + """Everything outside api_v3 still gets app.py's answer.""" + with web_app.app.test_request_context("/somewhere"): + resp, status = web_app.handle_exception(FORCED) + body = resp.get_json() + assert status == 500 + assert body == { + 'status': 'error', + 'error_code': 'UNKNOWN_ERROR', + 'message': 'An error occurred; see logs for details', + 'details': describe_exception(FORCED), + } + assert "SECRET123" not in body["details"] + + +class TestPluginActionStep1: + """execute_plugin_action's OAuth step-1 handler reports the script's error. + + The route bound a local `logger` in its JSON-parsing arm, which made + `logger` local to the whole function; every other `logger.error` in it + then raised UnboundLocalError. The step-1 handler therefore answered + "UnboundLocalError: cannot access local variable 'logger'" instead of + whatever the plugin's auth script actually raised. + """ + + @pytest.fixture + def plugin_dir(self, tmp_path): + import json + d = tmp_path / "demo-plugin" + d.mkdir() + (d / "manifest.json").write_text(json.dumps({ + "id": "demo-plugin", + "web_ui_actions": [{"id": "auth", "type": "script", + "script": "auth.py", "oauth_flow": True}], + }), encoding="utf-8") + (d / "auth.py").write_text( + "def get_auth_url():\n" + " raise RuntimeError('the auth script failed')\n", + encoding="utf-8") + return d + + def test_the_script_error_reaches_the_response(self, plugin_dir, monkeypatch): + from unittest.mock import MagicMock + manager = MagicMock() + manager.get_plugin_directory.return_value = str(plugin_dir) + monkeypatch.setattr(api_v3, "plugin_manager", manager, raising=False) + app = Flask(__name__) + app.register_blueprint(api_v3, url_prefix="/api/v3") + + resp = app.test_client().post( + "/api/v3/plugins/action", + json={"plugin_id": "demo-plugin", "action_id": "auth"}) + + assert resp.status_code == 500 + body = resp.get_json() + assert body["details"] == "RuntimeError: the auth script failed" + assert body["message"] == 'An error occurred; see logs for details' diff --git a/test/web_interface/test_error_handler.py b/test/web_interface/test_error_handler.py index ccbae7b6..74926c31 100644 --- a/test/web_interface/test_error_handler.py +++ b/test/web_interface/test_error_handler.py @@ -15,7 +15,7 @@ every api_v3 endpoint actually calls. import pytest from flask import Flask -from src.web_interface.api_helpers import success_response +from src.web_interface.api_helpers import exception_error_response, success_response from src.web_interface.error_handler import ( create_error_response, create_success_response, @@ -64,6 +64,59 @@ class TestCreateErrorResponse: assert response.get_json()["suggested_fixes"] == ["Try again"] +class TestExceptionErrorResponse: + """The one-call form of from_exception() + error_response(). + + Nine plugin routes spelled the pair out by hand; these pin that the helper + answers exactly what that spelling did, so folding them changed nothing a + client sees. + """ + + @staticmethod + def _by_hand(exc, code, with_context): + from src.web_interface.api_helpers import error_response + error = WebInterfaceError.from_exception(exc, code) + if with_context: + return error_response(error.error_code, error.message, + details=error.details, context=error.context, + status_code=500) + return error_response(error.error_code, error.message, + details=error.details, status_code=500) + + @pytest.mark.parametrize("with_context", [True, False]) + @pytest.mark.parametrize("code", [ErrorCode.SYSTEM_ERROR, + ErrorCode.CONFIG_SAVE_FAILED, + ErrorCode.PLUGIN_UPDATE_FAILED]) + def test_same_answer_as_the_hand_written_pair(self, app, code, with_context): + exc = ValueError("token=SECRET boom") + exc.context = {"config_path": "/etc/x.json"} + with app.test_request_context(): + got, got_status = exception_error_response( + exc, code, with_context=with_context) + want, want_status = self._by_hand(exc, code, with_context) + assert got_status == want_status == 500 + assert got.get_json() == want.get_json() + + def test_shape(self, app): + with app.test_request_context(): + response, status = exception_error_response( + RuntimeError("token=SECRET"), ErrorCode.SYSTEM_ERROR) + assert status == 500 + assert response.get_json() == { + "status": "error", + "error_code": "SYSTEM_ERROR", + "message": "A system error occurred", + "context": {"exception_type": "RuntimeError"}, + "suggested_fixes": ["Review error details and try again"], + } + + def test_without_context(self, app): + with app.test_request_context(): + response, _ = exception_error_response( + RuntimeError("x"), ErrorCode.SYSTEM_ERROR, with_context=False) + assert "context" not in response.get_json() + + class TestCreateSuccessResponse: def test_bare_success(self): assert create_success_response() == {"status": "success"} diff --git a/test/web_interface/test_errors.py b/test/web_interface/test_errors.py index d707f080..209c0e8b 100644 --- a/test/web_interface/test_errors.py +++ b/test/web_interface/test_errors.py @@ -1,7 +1,7 @@ """ Tests for src/web_interface/errors.py — the structured error type behind -every API error response (category inference, default suggestions, the -JSON shape, and exception conversion). +every API error response (default suggestions, the JSON shape, and +exception conversion). Pure logic; no Flask context needed. @@ -11,39 +11,7 @@ caller passing [] to mean "no suggestions" silently got the default list. import pytest -from src.web_interface.errors import ErrorCategory, ErrorCode, WebInterfaceError - - -class TestCategoryInference: - @pytest.mark.parametrize("code,expected", [ - (ErrorCode.CONFIG_SAVE_FAILED, ErrorCategory.CONFIGURATION), - (ErrorCode.CONFIG_ROLLBACK_FAILED, ErrorCategory.CONFIGURATION), - (ErrorCode.PLUGIN_NOT_FOUND, ErrorCategory.PLUGIN), - (ErrorCode.PLUGIN_OPERATION_CONFLICT, ErrorCategory.PLUGIN), - (ErrorCode.VALIDATION_ERROR, ErrorCategory.VALIDATION), - (ErrorCode.SCHEMA_VALIDATION_FAILED, ErrorCategory.VALIDATION), - (ErrorCode.INVALID_INPUT, ErrorCategory.VALIDATION), - (ErrorCode.NETWORK_ERROR, ErrorCategory.NETWORK), - (ErrorCode.API_ERROR, ErrorCategory.NETWORK), - (ErrorCode.TIMEOUT, ErrorCategory.NETWORK), - (ErrorCode.PERMISSION_DENIED, ErrorCategory.PERMISSION), - (ErrorCode.FILE_PERMISSION_ERROR, ErrorCategory.PERMISSION), - (ErrorCode.SYSTEM_ERROR, ErrorCategory.SYSTEM), - (ErrorCode.SERVICE_UNAVAILABLE, ErrorCategory.SYSTEM), - (ErrorCode.UNKNOWN_ERROR, ErrorCategory.UNKNOWN), - ]) - def test_every_code_prefix_maps_to_its_category(self, code, expected): - assert WebInterfaceError(code, "msg").category is expected - - def test_explicit_category_overrides_inference(self): - error = WebInterfaceError( - ErrorCode.CONFIG_SAVE_FAILED, "msg", category=ErrorCategory.SYSTEM) - assert error.category is ErrorCategory.SYSTEM - - def test_every_error_code_gets_a_category(self): - # No code may fall through uncategorized as the enum grows. - for code in ErrorCode: - assert isinstance(WebInterfaceError(code, "msg").category, ErrorCategory) +from src.web_interface.errors import ErrorCode, WebInterfaceError class TestDefaultSuggestions: @@ -79,8 +47,9 @@ class TestToDict: result = WebInterfaceError(ErrorCode.SYSTEM_ERROR, "boom").to_dict() assert result["status"] == "error" assert result["error_code"] == "SYSTEM_ERROR" - assert result["error_category"] == "system" assert result["message"] == "boom" + # No error_category: nothing in the UI, tests or plugins ever read it. + assert set(result) == {"status", "error_code", "message", "suggested_fixes"} def test_details_included_when_set(self): result = WebInterfaceError( @@ -116,23 +85,7 @@ class TestToDict: class TestFromException: - @pytest.mark.parametrize("exc_name,expected", [ - ("ConfigError", ErrorCode.CONFIG_LOAD_FAILED), - ("PluginError", ErrorCode.PLUGIN_LOAD_FAILED), - ("PermissionError", ErrorCode.PERMISSION_DENIED), - ("AccessDenied", ErrorCode.PERMISSION_DENIED), - ("ValidationError", ErrorCode.VALIDATION_ERROR), - ("SchemaError", ErrorCode.VALIDATION_ERROR), - ("NetworkError", ErrorCode.NETWORK_ERROR), - ("ConnectionError", ErrorCode.NETWORK_ERROR), - ("TimeoutError", ErrorCode.TIMEOUT), - ("SomethingElse", ErrorCode.UNKNOWN_ERROR), - ]) - def test_code_inferred_from_exception_class_name(self, exc_name, expected): - exc = type(exc_name, (Exception,), {})("boom") - assert WebInterfaceError.from_exception(exc).error_code is expected - - def test_explicit_code_skips_inference(self): + def test_the_given_code_is_reported(self): error = WebInterfaceError.from_exception( ValueError("boom"), error_code=ErrorCode.PLUGIN_NOT_FOUND) assert error.error_code is ErrorCode.PLUGIN_NOT_FOUND @@ -140,28 +93,28 @@ class TestFromException: def test_message_is_the_safe_one_not_the_exception_text(self): # The raw exception text is not echoed into `message`; that field is # a fixed, user-facing string per code. - error = WebInterfaceError.from_exception(ValueError("secret-ish detail")) + error = WebInterfaceError.from_exception(ValueError("secret-ish detail"), ErrorCode.UNKNOWN_ERROR) assert error.message == "An unexpected error occurred" assert "secret-ish" not in error.message def test_exception_type_recorded_in_context(self): - error = WebInterfaceError.from_exception(ValueError("boom")) + error = WebInterfaceError.from_exception(ValueError("boom"), ErrorCode.UNKNOWN_ERROR) assert error.context["exception_type"] == "ValueError" def test_caller_context_is_preserved_alongside_type(self): error = WebInterfaceError.from_exception( - ValueError("boom"), context={"plugin_id": "clock"}) + ValueError("boom"), ErrorCode.UNKNOWN_ERROR, context={"plugin_id": "clock"}) assert error.context["plugin_id"] == "clock" assert error.context["exception_type"] == "ValueError" def test_caller_supplied_exception_type_is_overwritten(self): error = WebInterfaceError.from_exception( - ValueError("boom"), context={"exception_type": "Fake"}) + ValueError("boom"), ErrorCode.UNKNOWN_ERROR, context={"exception_type": "Fake"}) assert error.context["exception_type"] == "ValueError" def test_original_error_retained(self): exc = ValueError("boom") - assert WebInterfaceError.from_exception(exc).original_error is exc + assert WebInterfaceError.from_exception(exc, ErrorCode.UNKNOWN_ERROR).original_error is exc def test_every_code_has_a_safe_message(self): for code in ErrorCode: @@ -205,4 +158,4 @@ class TestExceptionDetails: def test_details_flow_into_from_exception(self): exc = ValueError("boom") exc.context = {"config_path": "/etc/x.json"} - assert "config_path" in WebInterfaceError.from_exception(exc).details + assert "config_path" in WebInterfaceError.from_exception(exc, ErrorCode.UNKNOWN_ERROR).details diff --git a/web_interface/blueprints/api_v3/__init__.py b/web_interface/blueprints/api_v3/__init__.py index 29a9f4e4..4e5815df 100644 --- a/web_interface/blueprints/api_v3/__init__.py +++ b/web_interface/blueprints/api_v3/__init__.py @@ -35,13 +35,16 @@ from typing import Dict, Any, Optional, Tuple, Type from urllib.parse import urlparse, urlunparse logger = logging.getLogger(__name__) # Import new infrastructure -from src.web_interface.api_helpers import success_response, error_response, validate_request_json +from src.web_interface.api_helpers import (success_response, error_response, + exception_error_response, validate_request_json) from src.web_interface.errors import ErrorCode from src.web_interface.secret_helpers import (find_secret_fields, mask_all_secret_values, merge_secrets, remove_empty_secrets, separate_secrets, strip_masked_values) -from src.web_interface.error_handler import describe_exception, redact_text +from src.web_interface.error_handler import (describe_exception, http_exception_payload, + redact_text, unhandled_exception_payload) +from werkzeug.exceptions import HTTPException from src.plugin_system.operation_types import OperationType from src.web_interface.validators import ( validate_file_upload @@ -114,6 +117,43 @@ SYSTEM_FONTS = frozenset([ 'clr6x12', 'helvr12', 'texgyre-27' ]) api_v3 = Blueprint('api_v3', __name__) + + +@api_v3.errorhandler(Exception) +def _api_v3_unhandled_exception(error): + """The answer for any exception an api_v3 route does not handle itself. + + Fifty-odd routes used to end in the same four lines -- log the traceback, + return {status, message: "An error occurred; see logs for details", + details: describe_exception(e)} with a 500. This is those four lines, once. + A route still catches for itself when its failure needs something else: a + specific message, extra keys, an operation-history record, or cleanup. + + It is registered on the blueprint, not left to web_interface/app.py's + global handler, because the two answers differ: the global one adds + `error_code: UNKNOWN_ERROR`, and the plugin API client treats a body with + an error_code differently from one without (see api_client.js). Tests that + mount this blueprint on a bare Flask app get the same answer as the real + app does, too. + + `details` is describe_exception(), which redacts credentials and caps the + length. CodeQL reads returning it as stack-trace exposure; it is the + project's deliberate trade-off, because a device whose storage is failing + otherwise answers "see logs for details" from the log viewer too + (test_web_error_detail.py). + + Werkzeug's HTTPExceptions subclass Exception, so a 400/405/413/415 raised + inside a route lands here as well; it goes back as itself, in the global + handler's shape. A 404 or explicit 500 never arrives: Flask prefers the + app's code-specific handlers over a blueprint's class-based one. + """ + if isinstance(error, HTTPException): + return jsonify(http_exception_payload(error)), error.code or 500 + logger.error("Unhandled exception in %s", request.endpoint or request.path, + exc_info=error) + return jsonify(unhandled_exception_payload(error)), 500 + + def _get_plugin_version(plugin_id: str) -> str: """Read the installed version from a plugin's manifest.json. diff --git a/web_interface/blueprints/api_v3/config.py b/web_interface/blueprints/api_v3/config.py index 0cc3eeda..4cff4e51 100644 --- a/web_interface/blueprints/api_v3/config.py +++ b/web_interface/blueprints/api_v3/config.py @@ -47,15 +47,11 @@ def _day_setting(data, day, flat_key, nested_key): @api_v3.route('/config/main', methods=['GET']) def get_main_config(): """Get main configuration, with credentials redacted.""" - try: - if not api_v3.config_manager: - return jsonify({'status': 'error', 'message': 'Config manager not initialized'}), 500 + if not api_v3.config_manager: + return jsonify({'status': 'error', 'message': 'Config manager not initialized'}), 500 - config = api_v3.config_manager.load_config() - return jsonify({'status': 'success', 'data': _redact_credentials(config)}) - except Exception as e: - logger.error('Unhandled exception', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + config = api_v3.config_manager.load_config() + return jsonify({'status': 'success', 'data': _redact_credentials(config)}) @api_v3.route('/config/schedule', methods=['GET']) def get_schedule_config(): """Get current schedule configuration""" @@ -1165,20 +1161,16 @@ def save_main_config(): @api_v3.route('/config/secrets', methods=['GET']) def get_secrets_config(): """Get secrets configuration""" - try: - if not api_v3.config_manager: - return jsonify({'status': 'error', 'message': 'Config manager not initialized'}), 500 + if not api_v3.config_manager: + return jsonify({'status': 'error', 'message': 'Config manager not initialized'}), 500 - config = api_v3.config_manager.get_raw_file_content('secrets') - # This interface has no authentication, and this file is nothing but - # credentials. It was handing all of them to anyone who could reach - # the port. Values are masked; empty and YOUR_* placeholders are left - # alone so a client can still tell "set" from "not set". - return jsonify({'status': 'success', - 'data': mask_all_secret_values(config)}) - except Exception as e: - logger.error('Unhandled exception', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + config = api_v3.config_manager.get_raw_file_content('secrets') + # This interface has no authentication, and this file is nothing but + # credentials. It was handing all of them to anyone who could reach + # the port. Values are masked; empty and YOUR_* placeholders are left + # alone so a client can still tell "set" from "not set". + return jsonify({'status': 'success', + 'data': mask_all_secret_values(config)}) @api_v3.route('/config/raw/main', methods=['POST']) def save_raw_main_config(): """Save raw main configuration JSON""" diff --git a/web_interface/blueprints/api_v3/display.py b/web_interface/blueprints/api_v3/display.py index dcb69a50..68a0ab0d 100644 --- a/web_interface/blueprints/api_v3/display.py +++ b/web_interface/blueprints/api_v3/display.py @@ -6,7 +6,7 @@ endpoint names are unchanged by living here. from web_interface.blueprints.api_v3 import ( _ensure_display_service_running, _get_display_service_status, _stop_display_service, api_v3, - describe_exception, jsonify, logger, os, request, uuid, + jsonify, logger, os, request, uuid, ) import web_interface.blueprints.api_v3 as _pkg # Read through the module rather than bound by value: tests patch these @@ -31,45 +31,41 @@ def _cache_manager(): @api_v3.route('/display/current', methods=['GET']) def get_display_current(): """Get current display state""" + import base64 + from PIL import Image + import io + + snapshot_path = "/tmp/led_matrix_preview.png" + + # Get display dimensions from config: the logical size DisplayManager + # renders at, so double-sided setups preview one screen + from src.display_geometry import logical_size try: - import base64 - from PIL import Image - import io + config = api_v3.config_manager.load_config() if api_v3.config_manager else {} + width, height = logical_size(config) + except Exception: + width, height = logical_size({}) - snapshot_path = "/tmp/led_matrix_preview.png" - - # Get display dimensions from config: the logical size DisplayManager - # renders at, so double-sided setups preview one screen - from src.display_geometry import logical_size + # Try to read snapshot file + image_data = None + if os.path.exists(snapshot_path): try: - config = api_v3.config_manager.load_config() if api_v3.config_manager else {} - width, height = logical_size(config) - except Exception: - width, height = logical_size({}) + with Image.open(snapshot_path) as img: + # Convert to PNG and encode as base64 + buffer = io.BytesIO() + img.save(buffer, format='PNG') + image_data = base64.b64encode(buffer.getvalue()).decode('utf-8') + except Exception as img_err: + # File might be being written or corrupted, return None + pass - # Try to read snapshot file - image_data = None - if os.path.exists(snapshot_path): - try: - with Image.open(snapshot_path) as img: - # Convert to PNG and encode as base64 - buffer = io.BytesIO() - img.save(buffer, format='PNG') - image_data = base64.b64encode(buffer.getvalue()).decode('utf-8') - except Exception as img_err: - # File might be being written or corrupted, return None - pass - - display_data = { - 'timestamp': _pkg.time.time(), - 'width': width, - 'height': height, - 'image': image_data # Base64 encoded image data or None if unavailable - } - return jsonify({'status': 'success', 'data': display_data}) - except Exception as e: - logger.error('Unhandled exception', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + display_data = { + 'timestamp': _pkg.time.time(), + 'width': width, + 'height': height, + 'image': image_data # Base64 encoded image data or None if unavailable + } + return jsonify({'status': 'success', 'data': display_data}) @api_v3.route('/display/modes', methods=['GET']) def get_display_modes(): """Every display mode that can be requested on-demand, with its plugin. @@ -92,230 +88,208 @@ def get_display_modes(): for the duration -- so they are reported with enabled: false rather than omitted. """ - try: - if not api_v3.plugin_manager: - return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 + if not api_v3.plugin_manager: + return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 - # Discovery is lazy and normally triggered by whichever endpoint runs - # first, which is a person opening the dashboard. A caller that never - # visits it would otherwise see an empty list. - api_v3.plugin_manager.discover_plugins() + # Discovery is lazy and normally triggered by whichever endpoint runs + # first, which is a person opening the dashboard. A caller that never + # visits it would otherwise see an empty list. + api_v3.plugin_manager.discover_plugins() - include_disabled = request.args.get('include_disabled') in ('1', 'true', 'True') - full_config = api_v3.config_manager.load_config() if api_v3.config_manager else {} + include_disabled = request.args.get('include_disabled') in ('1', 'true', 'True') + full_config = api_v3.config_manager.load_config() if api_v3.config_manager else {} - modes = [] - for plugin_id, manifest in sorted(api_v3.plugin_manager.plugin_manifests.items()): - # A hand-edited or migrated config.json can hold a non-dict under a - # plugin id; DisplayController._reconcile guards the same shape, so - # it happens in practice. Without this, .get() raises AttributeError, - # the loop aborts and the endpoint answers 500 with no modes at all - # -- one bad section would blank every entity the MQTT bridge builds - # from this list. - plugin_config = full_config.get(plugin_id) - if not isinstance(plugin_config, dict): - if plugin_config is not None: - logger.warning( - "Config for plugin %r is %s, not an object; treating it as disabled", - plugin_id, type(plugin_config).__name__) - plugin_config = {} - enabled = bool(plugin_config.get('enabled', False)) - if not enabled and not include_disabled: - continue - plugin_name = (manifest or {}).get('name') or plugin_id - plugin_modes = api_v3.plugin_manager.get_plugin_display_modes(plugin_id) or [plugin_id] - for mode in plugin_modes: - # A single-mode plugin's mode is the plugin, so its own name is - # the readable label. Multi-mode plugins have no per-mode name - # anywhere, so the raw mode string is the only thing to show. - modes.append({ - 'mode': mode, - 'plugin_id': plugin_id, - 'plugin_name': plugin_name, - 'name': plugin_name if len(plugin_modes) == 1 else mode, - 'enabled': enabled, - }) + modes = [] + for plugin_id, manifest in sorted(api_v3.plugin_manager.plugin_manifests.items()): + # A hand-edited or migrated config.json can hold a non-dict under a + # plugin id; DisplayController._reconcile guards the same shape, so + # it happens in practice. Without this, .get() raises AttributeError, + # the loop aborts and the endpoint answers 500 with no modes at all + # -- one bad section would blank every entity the MQTT bridge builds + # from this list. + plugin_config = full_config.get(plugin_id) + if not isinstance(plugin_config, dict): + if plugin_config is not None: + logger.warning( + "Config for plugin %r is %s, not an object; treating it as disabled", + plugin_id, type(plugin_config).__name__) + plugin_config = {} + enabled = bool(plugin_config.get('enabled', False)) + if not enabled and not include_disabled: + continue + plugin_name = (manifest or {}).get('name') or plugin_id + plugin_modes = api_v3.plugin_manager.get_plugin_display_modes(plugin_id) or [plugin_id] + for mode in plugin_modes: + # A single-mode plugin's mode is the plugin, so its own name is + # the readable label. Multi-mode plugins have no per-mode name + # anywhere, so the raw mode string is the only thing to show. + modes.append({ + 'mode': mode, + 'plugin_id': plugin_id, + 'plugin_name': plugin_name, + 'name': plugin_name if len(plugin_modes) == 1 else mode, + 'enabled': enabled, + }) - return jsonify({'status': 'success', 'data': {'modes': modes}}) - except Exception as exc: - # describe_exception, not a bare message: test_web_error_detail.py - # enforces that every handler here returns it, because a device whose - # storage is failing otherwise answers "see logs for details" from the - # log viewer too. It redacts credentials out of the exception text. - # CodeQL flags this as stack-trace exposure across all ~75 handlers; - # it is the project's deliberate, reviewed trade-off. - logger.error('Error in get_display_modes', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(exc)}), 500 + return jsonify({'status': 'success', 'data': {'modes': modes}}) @api_v3.route('/display/on-demand/status', methods=['GET']) def get_on_demand_status(): """Return the current on-demand display state.""" - try: - cache = _cache_manager() - # memory_ttl=0: the display service writes this key, so only the file - # is current. This process's memory tier would keep serving the first - # copy it read for the full max_age -- "active" for two minutes after - # the display had already stopped. - state = cache.get('display_on_demand_state', max_age=120, memory_ttl=0) - if state is None: - state = { - 'active': False, - 'status': 'idle', - 'last_updated': None - } - service_status = _get_display_service_status() - return jsonify({ - 'status': 'success', - 'data': { - 'state': state, - 'service': service_status - } - }) - except Exception as exc: - logger.error('Error in get_on_demand_status', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(exc)}), 500 + cache = _cache_manager() + # memory_ttl=0: the display service writes this key, so only the file + # is current. This process's memory tier would keep serving the first + # copy it read for the full max_age -- "active" for two minutes after + # the display had already stopped. + state = cache.get('display_on_demand_state', max_age=120, memory_ttl=0) + if state is None: + state = { + 'active': False, + 'status': 'idle', + 'last_updated': None + } + service_status = _get_display_service_status() + return jsonify({ + 'status': 'success', + 'data': { + 'state': state, + 'service': service_status + } + }) @api_v3.route('/display/on-demand/start', methods=['POST']) def start_on_demand_display(): """Request the display controller to run a specific plugin on-demand.""" - try: - data = request.get_json(silent=True) or {} - plugin_id = data.get('plugin_id') - mode = data.get('mode') - duration = data.get('duration') - pinned = bool(data.get('pinned', False)) - start_service = data.get('start_service', True) + data = request.get_json(silent=True) or {} + plugin_id = data.get('plugin_id') + mode = data.get('mode') + duration = data.get('duration') + pinned = bool(data.get('pinned', False)) + start_service = data.get('start_service', True) - if not plugin_id and not mode: - return jsonify({'status': 'error', 'message': 'plugin_id or mode is required'}), 400 + if not plugin_id and not mode: + return jsonify({'status': 'error', 'message': 'plugin_id or mode is required'}), 400 - resolved_plugin = plugin_id - resolved_mode = mode + resolved_plugin = plugin_id + resolved_mode = mode - if api_v3.plugin_manager: - if resolved_plugin and resolved_plugin not in _pkg._discovered_plugin_manifests(resolved_plugin): - return jsonify({'status': 'error', 'message': f'Plugin {resolved_plugin} not found'}), 404 + if api_v3.plugin_manager: + if resolved_plugin and resolved_plugin not in _pkg._discovered_plugin_manifests(resolved_plugin): + return jsonify({'status': 'error', 'message': f'Plugin {resolved_plugin} not found'}), 404 - if resolved_plugin and not resolved_mode: - modes = api_v3.plugin_manager.get_plugin_display_modes(resolved_plugin) - resolved_mode = modes[0] if modes else resolved_plugin - elif resolved_mode and not resolved_plugin: - _pkg._discovered_plugin_manifests() + if resolved_plugin and not resolved_mode: + modes = api_v3.plugin_manager.get_plugin_display_modes(resolved_plugin) + resolved_mode = modes[0] if modes else resolved_plugin + elif resolved_mode and not resolved_plugin: + _pkg._discovered_plugin_manifests() + resolved_plugin = api_v3.plugin_manager.find_plugin_for_mode(resolved_mode) + if not resolved_plugin: + # Not among what was discovered: the plugin that declares + # it may have been installed since. Scan once more. + _pkg._discovered_plugin_manifests(rescan=True) resolved_plugin = api_v3.plugin_manager.find_plugin_for_mode(resolved_mode) - if not resolved_plugin: - # Not among what was discovered: the plugin that declares - # it may have been installed since. Scan once more. - _pkg._discovered_plugin_manifests(rescan=True) - resolved_plugin = api_v3.plugin_manager.find_plugin_for_mode(resolved_mode) - if not resolved_plugin: - return jsonify({'status': 'error', 'message': f'Mode {resolved_mode} not found'}), 404 + if not resolved_plugin: + return jsonify({'status': 'error', 'message': f'Mode {resolved_mode} not found'}), 404 - # Note: On-demand can work with disabled plugins - the display controller - # will temporarily enable them during initialization if needed - # We don't block the request here, but log it for debugging - if api_v3.config_manager and resolved_plugin: - config = api_v3.config_manager.load_config() - plugin_config = config.get(resolved_plugin, {}) - if 'enabled' in plugin_config and not plugin_config.get('enabled', False): - logger.info( - "On-demand request for disabled plugin '%s' - will be temporarily enabled", - resolved_plugin, - ) + # Note: On-demand can work with disabled plugins - the display controller + # will temporarily enable them during initialization if needed + # We don't block the request here, but log it for debugging + if api_v3.config_manager and resolved_plugin: + config = api_v3.config_manager.load_config() + plugin_config = config.get(resolved_plugin, {}) + if 'enabled' in plugin_config and not plugin_config.get('enabled', False): + logger.info( + "On-demand request for disabled plugin '%s' - will be temporarily enabled", + resolved_plugin, + ) - # Set the on-demand request in cache FIRST (before starting service) - # This ensures the request is available when the service starts/restarts - cache = _cache_manager() - request_id = data.get('request_id') or str(uuid.uuid4()) - request_payload = { - 'request_id': request_id, - 'action': 'start', - 'plugin_id': resolved_plugin, - 'mode': resolved_mode, - 'duration': duration, - 'pinned': pinned, - 'timestamp': _pkg.time.time() - } - cache.set('display_on_demand_request', request_payload) + # Set the on-demand request in cache FIRST (before starting service) + # This ensures the request is available when the service starts/restarts + cache = _cache_manager() + request_id = data.get('request_id') or str(uuid.uuid4()) + request_payload = { + 'request_id': request_id, + 'action': 'start', + 'plugin_id': resolved_plugin, + 'mode': resolved_mode, + 'duration': duration, + 'pinned': pinned, + 'timestamp': _pkg.time.time() + } + cache.set('display_on_demand_request', request_payload) - # Check if display service is running (or will be started) - service_status = _get_display_service_status() - service_was_running = service_status.get('active', False) + # Check if display service is running (or will be started) + service_status = _get_display_service_status() + service_was_running = service_status.get('active', False) - # Stop the display service first to ensure clean state when we will restart it - if service_was_running and start_service: - import time as time_module - logger.debug("Stopping display service before starting on-demand mode") - _stop_display_service() - # Wait a brief moment for the service to fully stop - time_module.sleep(1.5) - logger.debug("Display service stopped, now starting with on-demand request") + # Stop the display service first to ensure clean state when we will restart it + if service_was_running and start_service: + import time as time_module + logger.debug("Stopping display service before starting on-demand mode") + _stop_display_service() + # Wait a brief moment for the service to fully stop + time_module.sleep(1.5) + logger.debug("Display service stopped, now starting with on-demand request") - if not service_status.get('active') and not start_service: + if not service_status.get('active') and not start_service: + return jsonify({ + 'status': 'error', + 'message': 'Display service is not running. Please start the display service or enable "Start Service" option.', + 'service_status': service_status + }), 400 + + service_result = None + if start_service: + service_result = _ensure_display_service_running() + # Check if service actually started + if service_result and not service_result.get('active'): return jsonify({ 'status': 'error', - 'message': 'Display service is not running. Please start the display service or enable "Start Service" option.', - 'service_status': service_status - }), 400 - - service_result = None - if start_service: - service_result = _ensure_display_service_running() - # Check if service actually started - if service_result and not service_result.get('active'): - return jsonify({ - 'status': 'error', - 'message': 'Failed to start display service. Please check service logs or start it manually.', - 'service_result': service_result - }), 500 + 'message': 'Failed to start display service. Please check service logs or start it manually.', + 'service_result': service_result + }), 500 - # Service was restarted (or started fresh) with on-demand request in cache - # The display controller will read the request during initialization or when it polls + # Service was restarted (or started fresh) with on-demand request in cache + # The display controller will read the request during initialization or when it polls - response_data = { - 'request_id': request_id, - 'plugin_id': resolved_plugin, - 'mode': resolved_mode, - 'duration': duration, - 'pinned': pinned, - 'service': service_result - } - return jsonify({'status': 'success', 'data': response_data}) - except Exception as exc: - logger.error('Error in start_on_demand_display', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(exc)}), 500 + response_data = { + 'request_id': request_id, + 'plugin_id': resolved_plugin, + 'mode': resolved_mode, + 'duration': duration, + 'pinned': pinned, + 'service': service_result + } + return jsonify({'status': 'success', 'data': response_data}) @api_v3.route('/display/on-demand/stop', methods=['POST']) def stop_on_demand_display(): """Request the display controller to stop on-demand mode.""" - try: - data = request.get_json(silent=True) or {} - stop_service = data.get('stop_service', False) + data = request.get_json(silent=True) or {} + stop_service = data.get('stop_service', False) - # Set the stop request in cache FIRST - # The display controller will poll this and restart without the on-demand filter - cache = _cache_manager() - request_id = data.get('request_id') or str(uuid.uuid4()) - request_payload = { + # Set the stop request in cache FIRST + # The display controller will poll this and restart without the on-demand filter + cache = _cache_manager() + request_id = data.get('request_id') or str(uuid.uuid4()) + request_payload = { + 'request_id': request_id, + 'action': 'stop', + 'timestamp': _pkg.time.time() + } + cache.set('display_on_demand_request', request_payload) + + # Note: The display controller's _clear_on_demand() will handle the restart + # to restore normal operation with all plugins + + service_result = None + if stop_service: + service_result = _stop_display_service() + + return jsonify({ + 'status': 'success', + 'data': { 'request_id': request_id, - 'action': 'stop', - 'timestamp': _pkg.time.time() + 'service': service_result } - cache.set('display_on_demand_request', request_payload) - - # Note: The display controller's _clear_on_demand() will handle the restart - # to restore normal operation with all plugins - - service_result = None - if stop_service: - service_result = _stop_display_service() - - return jsonify({ - 'status': 'success', - 'data': { - 'request_id': request_id, - 'service': service_result - } - }) - except Exception as exc: - logger.error('Error in stop_on_demand_display', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(exc)}), 500 + }) @api_v3.route('/display/current-status', methods=['GET']) def get_current_display_status(): """Return the display mode/plugin currently intended to be shown. @@ -325,17 +299,13 @@ def get_current_display_status(): System Logs page) can show what's on screen without querying the display process directly. """ - try: - cache = _cache_manager() - # memory_ttl=0: written by the display service; see get_on_demand_status. - state = cache.get('display_current_state', max_age=120, memory_ttl=0) - if state is None: - state = { - 'mode': None, - 'plugin_id': None, - 'last_updated': None, - } - return jsonify({'status': 'success', 'data': state}) - except Exception as e: - logger.error('Error in get_current_display_status', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + cache = _cache_manager() + # memory_ttl=0: written by the display service; see get_on_demand_status. + state = cache.get('display_current_state', max_age=120, memory_ttl=0) + if state is None: + state = { + 'mode': None, + 'plugin_id': None, + 'last_updated': None, + } + return jsonify({'status': 'success', 'data': state}) diff --git a/web_interface/blueprints/api_v3/fonts.py b/web_interface/blueprints/api_v3/fonts.py index 2da6d753..ff52c744 100644 --- a/web_interface/blueprints/api_v3/fonts.py +++ b/web_interface/blueprints/api_v3/fonts.py @@ -4,7 +4,7 @@ Routes decorate the shared `api_v3` Blueprint from ._common, so their endpoint names are unchanged by living here. """ from web_interface.blueprints.api_v3 import ( - PROJECT_ROOT, Path, Response, SYSTEM_FONTS, api_v3, describe_exception, + PROJECT_ROOT, Path, Response, SYSTEM_FONTS, api_v3, jsonify, logger, os, re, request, validate_file_upload, ) @@ -52,439 +52,417 @@ def _catalog_response(catalog): @api_v3.route('/fonts/catalog', methods=['GET']) def get_fonts_catalog(): """Get fonts catalog""" + # Check cache first (5 minute TTL) try: - # Check cache first (5 minute TTL) + from web_interface.cache import get_cached, set_cached + cached_result = get_cached('fonts_catalog', ttl_seconds=300) + if cached_result is not None: + return _catalog_response(cached_result) + except ImportError: + # Cache not available, continue without caching + get_cached = None + set_cached = None + + # Try to import freetype, but continue without it if unavailable + try: + import freetype + freetype_available = True + except ImportError: + freetype_available = False + + # Scan assets/fonts directory for actual font files + fonts_dir = PROJECT_ROOT / "assets" / "fonts" + catalog = {} + + if fonts_dir.exists() and fonts_dir.is_dir(): + for filename in os.listdir(fonts_dir): + if filename.endswith(('.ttf', '.otf', '.bdf')): + filepath = fonts_dir / filename + # Generate family name from filename (without extension) + family_name = os.path.splitext(filename)[0] + + # Try to get font metadata using freetype (for TTF/OTF) + metadata = {} + if filename.endswith(('.ttf', '.otf')) and freetype_available: + try: + face = freetype.Face(str(filepath)) + if face.valid: + # Get font family name from font file + family_name_from_font = face.family_name.decode('utf-8') if face.family_name else family_name + metadata = { + 'family': family_name_from_font, + 'style': face.style_name.decode('utf-8') if face.style_name else 'Regular', + 'num_glyphs': face.num_glyphs, + 'units_per_em': face.units_per_EM + } + # Use font's family name if available + if family_name_from_font: + family_name = family_name_from_font + except Exception: + # If freetype fails, use filename-based name + pass + + # Store relative path from project root + relative_path = str(filepath.relative_to(PROJECT_ROOT)) + font_type = 'ttf' if filename.endswith('.ttf') else 'otf' if filename.endswith('.otf') else 'bdf' + + # Generate human-readable display name from family_name + display_name = family_name.replace('-', ' ').replace('_', ' ') + # Add space before capital letters for camelCase names + display_name = re.sub(r'([a-z])([A-Z])', r'\1 \2', display_name) + # Add space before numbers that follow letters + display_name = re.sub(r'([a-zA-Z])(\d)', r'\1 \2', display_name) + # Clean up multiple spaces + display_name = ' '.join(display_name.split()) + + # Use filename (without extension) as unique key to avoid collisions + # when multiple files share the same family_name from font metadata + catalog_key = os.path.splitext(filename)[0] + + # Check if this is a system font (cannot be deleted) + is_system = catalog_key.lower() in SYSTEM_FONTS + + # BDF files are fixed-size bitmap strikes: FreeType + # accepts only the pixel size baked into the file. The + # UI needs to know that before offering a size control, + # or it offers a number that cannot take effect. + native_size = None + if font_type == 'bdf': + try: + from src.element_style import _read_bdf_native_size + native_size = _read_bdf_native_size(str(filepath)) + except Exception as e: + logger.debug("Could not read native size for BDF font %s: %s", + filepath, e) + native_size = None + + catalog[catalog_key] = { + 'filename': filename, + 'family_name': family_name, + 'display_name': display_name, + 'path': relative_path, + 'type': font_type, + 'is_system': is_system, + 'scalable': font_type != 'bdf', + 'native_size': native_size, + 'metadata': metadata if metadata else None + } + + # Cache the result (5 minute TTL) if available + if set_cached: try: - from web_interface.cache import get_cached, set_cached - cached_result = get_cached('fonts_catalog', ttl_seconds=300) - if cached_result is not None: - return _catalog_response(cached_result) - except ImportError: - # Cache not available, continue without caching - get_cached = None - set_cached = None + set_cached('fonts_catalog', catalog, ttl_seconds=300) + except Exception: + logger.error("[FontCatalog] Failed to cache fonts_catalog", exc_info=True) - # Try to import freetype, but continue without it if unavailable - try: - import freetype - freetype_available = True - except ImportError: - freetype_available = False - - # Scan assets/fonts directory for actual font files - fonts_dir = PROJECT_ROOT / "assets" / "fonts" - catalog = {} - - if fonts_dir.exists() and fonts_dir.is_dir(): - for filename in os.listdir(fonts_dir): - if filename.endswith(('.ttf', '.otf', '.bdf')): - filepath = fonts_dir / filename - # Generate family name from filename (without extension) - family_name = os.path.splitext(filename)[0] - - # Try to get font metadata using freetype (for TTF/OTF) - metadata = {} - if filename.endswith(('.ttf', '.otf')) and freetype_available: - try: - face = freetype.Face(str(filepath)) - if face.valid: - # Get font family name from font file - family_name_from_font = face.family_name.decode('utf-8') if face.family_name else family_name - metadata = { - 'family': family_name_from_font, - 'style': face.style_name.decode('utf-8') if face.style_name else 'Regular', - 'num_glyphs': face.num_glyphs, - 'units_per_em': face.units_per_EM - } - # Use font's family name if available - if family_name_from_font: - family_name = family_name_from_font - except Exception: - # If freetype fails, use filename-based name - pass - - # Store relative path from project root - relative_path = str(filepath.relative_to(PROJECT_ROOT)) - font_type = 'ttf' if filename.endswith('.ttf') else 'otf' if filename.endswith('.otf') else 'bdf' - - # Generate human-readable display name from family_name - display_name = family_name.replace('-', ' ').replace('_', ' ') - # Add space before capital letters for camelCase names - display_name = re.sub(r'([a-z])([A-Z])', r'\1 \2', display_name) - # Add space before numbers that follow letters - display_name = re.sub(r'([a-zA-Z])(\d)', r'\1 \2', display_name) - # Clean up multiple spaces - display_name = ' '.join(display_name.split()) - - # Use filename (without extension) as unique key to avoid collisions - # when multiple files share the same family_name from font metadata - catalog_key = os.path.splitext(filename)[0] - - # Check if this is a system font (cannot be deleted) - is_system = catalog_key.lower() in SYSTEM_FONTS - - # BDF files are fixed-size bitmap strikes: FreeType - # accepts only the pixel size baked into the file. The - # UI needs to know that before offering a size control, - # or it offers a number that cannot take effect. - native_size = None - if font_type == 'bdf': - try: - from src.element_style import _read_bdf_native_size - native_size = _read_bdf_native_size(str(filepath)) - except Exception as e: - logger.debug("Could not read native size for BDF font %s: %s", - filepath, e) - native_size = None - - catalog[catalog_key] = { - 'filename': filename, - 'family_name': family_name, - 'display_name': display_name, - 'path': relative_path, - 'type': font_type, - 'is_system': is_system, - 'scalable': font_type != 'bdf', - 'native_size': native_size, - 'metadata': metadata if metadata else None - } - - # Cache the result (5 minute TTL) if available - if set_cached: - try: - set_cached('fonts_catalog', catalog, ttl_seconds=300) - except Exception: - logger.error("[FontCatalog] Failed to cache fonts_catalog", exc_info=True) - - return _catalog_response(catalog) - except Exception as e: - logger.error("%s failed", request.path, exc_info=True) - return jsonify({'status': 'error', - 'message': 'An error occurred; see logs for details', - 'details': describe_exception(e)}), 500 + return _catalog_response(catalog) @api_v3.route('/fonts/tokens', methods=['GET']) def get_font_tokens(): """Get font size tokens""" - try: - # This would integrate with the actual font system - # For now, return sample tokens - tokens = { - 'xs': 6, - 'sm': 8, - 'md': 10, - 'lg': 12, - 'xl': 14, - 'xxl': 16 - } - return jsonify({'status': 'success', 'data': {'tokens': tokens}}) - except Exception as e: - logger.error('Unhandled exception', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + # This would integrate with the actual font system + # For now, return sample tokens + tokens = { + 'xs': 6, + 'sm': 8, + 'md': 10, + 'lg': 12, + 'xl': 14, + 'xxl': 16 + } + return jsonify({'status': 'success', 'data': {'tokens': tokens}}) @api_v3.route('/fonts/upload', methods=['POST']) def upload_font(): """Upload font file""" - try: - if 'font_file' not in request.files: - return jsonify({'status': 'error', 'message': 'No font file provided'}), 400 + if 'font_file' not in request.files: + return jsonify({'status': 'error', 'message': 'No font file provided'}), 400 - font_file = request.files['font_file'] - if font_file.filename == '': - return jsonify({'status': 'error', 'message': 'No file selected'}), 400 + font_file = request.files['font_file'] + if font_file.filename == '': + return jsonify({'status': 'error', 'message': 'No file selected'}), 400 - # Validate filename. validate_file_upload takes max_size_mb but only - # checks the filename/extension with it -- it never looks at the - # actual upload size, so the size limit below is enforced separately - # before the file is saved (same pattern as the .star upload above). - MAX_FONT_SIZE_MB = 10 - is_valid, error_msg = validate_file_upload( - font_file.filename, - max_size_mb=MAX_FONT_SIZE_MB, - allowed_extensions=['.ttf', '.otf', '.bdf'] - ) - if not is_valid: - return jsonify({'status': 'error', 'message': error_msg}), 400 - - # Check file size (stated limit is MAX_FONT_SIZE_MB) - font_file.seek(0, 2) # Seek to end - file_size = font_file.tell() - font_file.seek(0) # Reset to beginning - max_font_size_bytes = MAX_FONT_SIZE_MB * 1024 * 1024 - if file_size > max_font_size_bytes: - return jsonify({ - 'status': 'error', - 'message': f'File too large (max {MAX_FONT_SIZE_MB}MB, got {file_size / 1024 / 1024:.1f}MB)' - }), 400 - - font_family = request.form.get('font_family', '') - - if not font_family: - return jsonify({'status': 'error', 'message': 'Font file and family name required'}), 400 - - # Validate font family name - if not font_family.replace('_', '').replace('-', '').isalnum(): - return jsonify({'status': 'error', 'message': 'Font family name must contain only letters, numbers, underscores, and hyphens'}), 400 - - # Save the font file to assets/fonts directory - fonts_dir = PROJECT_ROOT / "assets" / "fonts" - fonts_dir.mkdir(parents=True, exist_ok=True) - - # Create filename from family name - original_ext = os.path.splitext(font_file.filename)[1].lower() - safe_filename = f"{font_family}{original_ext}" - filepath = fonts_dir / safe_filename - - # Check if file already exists - if filepath.exists(): - return jsonify({'status': 'error', 'message': f'Font with name {font_family} already exists'}), 400 - - # Save the file - font_file.save(str(filepath)) - - # Clear font catalog cache - try: - from web_interface.cache import delete_cached - delete_cached('fonts_catalog') - except ImportError as e: - logger.warning("[FontUpload] Cache module not available: %s", e) - except Exception: - logger.error("[FontUpload] Failed to clear fonts_catalog cache", exc_info=True) + # Validate filename. validate_file_upload takes max_size_mb but only + # checks the filename/extension with it -- it never looks at the + # actual upload size, so the size limit below is enforced separately + # before the file is saved (same pattern as the .star upload above). + MAX_FONT_SIZE_MB = 10 + is_valid, error_msg = validate_file_upload( + font_file.filename, + max_size_mb=MAX_FONT_SIZE_MB, + allowed_extensions=['.ttf', '.otf', '.bdf'] + ) + if not is_valid: + return jsonify({'status': 'error', 'message': error_msg}), 400 + # Check file size (stated limit is MAX_FONT_SIZE_MB) + font_file.seek(0, 2) # Seek to end + file_size = font_file.tell() + font_file.seek(0) # Reset to beginning + max_font_size_bytes = MAX_FONT_SIZE_MB * 1024 * 1024 + if file_size > max_font_size_bytes: return jsonify({ - 'status': 'success', - 'message': f'Font {font_family} uploaded successfully', - 'font_family': font_family, - 'filename': safe_filename, - 'path': f'assets/fonts/{safe_filename}' - }) - except Exception as e: - logger.error('Unhandled exception', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + 'status': 'error', + 'message': f'File too large (max {MAX_FONT_SIZE_MB}MB, got {file_size / 1024 / 1024:.1f}MB)' + }), 400 + + font_family = request.form.get('font_family', '') + + if not font_family: + return jsonify({'status': 'error', 'message': 'Font file and family name required'}), 400 + + # Validate font family name + if not font_family.replace('_', '').replace('-', '').isalnum(): + return jsonify({'status': 'error', 'message': 'Font family name must contain only letters, numbers, underscores, and hyphens'}), 400 + + # Save the font file to assets/fonts directory + fonts_dir = PROJECT_ROOT / "assets" / "fonts" + fonts_dir.mkdir(parents=True, exist_ok=True) + + # Create filename from family name + original_ext = os.path.splitext(font_file.filename)[1].lower() + safe_filename = f"{font_family}{original_ext}" + filepath = fonts_dir / safe_filename + + # Check if file already exists + if filepath.exists(): + return jsonify({'status': 'error', 'message': f'Font with name {font_family} already exists'}), 400 + + # Save the file + font_file.save(str(filepath)) + + # Clear font catalog cache + try: + from web_interface.cache import delete_cached + delete_cached('fonts_catalog') + except ImportError as e: + logger.warning("[FontUpload] Cache module not available: %s", e) + except Exception: + logger.error("[FontUpload] Failed to clear fonts_catalog cache", exc_info=True) + + return jsonify({ + 'status': 'success', + 'message': f'Font {font_family} uploaded successfully', + 'font_family': font_family, + 'filename': safe_filename, + 'path': f'assets/fonts/{safe_filename}' + }) @api_v3.route('/fonts/preview', methods=['GET']) def get_font_preview() -> tuple[Response, int] | Response: """Generate a preview image of text rendered with a specific font""" + from PIL import Image, ImageDraw, ImageFont + import io + import base64 + + # Limits to prevent DoS via large image generation on constrained devices + MAX_TEXT_CHARS = 100 + MAX_TEXT_LINES = 3 + MAX_DIM = 1024 # Max width or height in pixels + MAX_PIXELS = 500000 # Max total pixels (e.g., ~700x700) + + font_filename = request.args.get('font', '') + text = request.args.get('text', 'Sample Text 123') + bg_color = request.args.get('bg', '000000') + fg_color = request.args.get('fg', 'ffffff') + + # Validate text length and line count early + if len(text) > MAX_TEXT_CHARS: + return jsonify({'status': 'error', 'message': f'Text exceeds maximum length of {MAX_TEXT_CHARS} characters'}), 400 + if text.count('\n') >= MAX_TEXT_LINES: + return jsonify({'status': 'error', 'message': f'Text exceeds maximum of {MAX_TEXT_LINES} lines'}), 400 + + # Safe integer parsing for size try: - from PIL import Image, ImageDraw, ImageFont - import io - import base64 + size = int(request.args.get('size', 12)) + except (ValueError, TypeError, OverflowError): + return jsonify({'status': 'error', 'message': 'Invalid font size'}), 400 - # Limits to prevent DoS via large image generation on constrained devices - MAX_TEXT_CHARS = 100 - MAX_TEXT_LINES = 3 - MAX_DIM = 1024 # Max width or height in pixels - MAX_PIXELS = 500000 # Max total pixels (e.g., ~700x700) + if not font_filename: + return jsonify({'status': 'error', 'message': 'Font filename required'}), 400 - font_filename = request.args.get('font', '') - text = request.args.get('text', 'Sample Text 123') - bg_color = request.args.get('bg', '000000') - fg_color = request.args.get('fg', 'ffffff') + # Validate size + if size < 4 or size > 72: + return jsonify({'status': 'error', 'message': 'Font size must be between 4 and 72'}), 400 - # Validate text length and line count early - if len(text) > MAX_TEXT_CHARS: - return jsonify({'status': 'error', 'message': f'Text exceeds maximum length of {MAX_TEXT_CHARS} characters'}), 400 - if text.count('\n') >= MAX_TEXT_LINES: - return jsonify({'status': 'error', 'message': f'Text exceeds maximum of {MAX_TEXT_LINES} lines'}), 400 + # Security: Validate font_filename to prevent path traversal + # Only allow alphanumeric, hyphen, underscore, and dot (for extension) + safe_name = Path(font_filename).name # Strip any directory components + if safe_name != font_filename or '..' in font_filename: + return jsonify({'status': 'error', 'message': 'Invalid font filename'}), 400 - # Safe integer parsing for size - try: - size = int(request.args.get('size', 12)) - except (ValueError, TypeError, OverflowError): - return jsonify({'status': 'error', 'message': 'Invalid font size'}), 400 + # Validate extension + allowed_extensions = ['.ttf', '.otf', '.bdf'] + has_valid_ext = any(safe_name.lower().endswith(ext) for ext in allowed_extensions) + name_without_ext = safe_name.rsplit('.', 1)[0] if '.' in safe_name else safe_name - if not font_filename: - return jsonify({'status': 'error', 'message': 'Font filename required'}), 400 + # Find the font file + fonts_dir = PROJECT_ROOT / "assets" / "fonts" + if not fonts_dir.exists(): + return jsonify({'status': 'error', 'message': 'Fonts directory not found'}), 404 - # Validate size - if size < 4 or size > 72: - return jsonify({'status': 'error', 'message': 'Font size must be between 4 and 72'}), 400 + font_path = fonts_dir / safe_name - # Security: Validate font_filename to prevent path traversal - # Only allow alphanumeric, hyphen, underscore, and dot (for extension) - safe_name = Path(font_filename).name # Strip any directory components - if safe_name != font_filename or '..' in font_filename: - return jsonify({'status': 'error', 'message': 'Invalid font filename'}), 400 + if not font_path.exists() and not has_valid_ext: + # Try finding by family name (without extension) + for ext in allowed_extensions: + potential_path = fonts_dir / f"{name_without_ext}{ext}" + if potential_path.exists(): + font_path = potential_path + break - # Validate extension - allowed_extensions = ['.ttf', '.otf', '.bdf'] - has_valid_ext = any(safe_name.lower().endswith(ext) for ext in allowed_extensions) - name_without_ext = safe_name.rsplit('.', 1)[0] if '.' in safe_name else safe_name + # Final security check: ensure path is within fonts_dir + try: + font_path.resolve().relative_to(fonts_dir.resolve()) + except ValueError: + return jsonify({'status': 'error', 'message': 'Invalid font path'}), 400 - # Find the font file - fonts_dir = PROJECT_ROOT / "assets" / "fonts" - if not fonts_dir.exists(): - return jsonify({'status': 'error', 'message': 'Fonts directory not found'}), 404 + if not font_path.exists(): + return jsonify({'status': 'error', 'message': f'Font file not found: {font_filename}'}), 404 - font_path = fonts_dir / safe_name - - if not font_path.exists() and not has_valid_ext: - # Try finding by family name (without extension) - for ext in allowed_extensions: - potential_path = fonts_dir / f"{name_without_ext}{ext}" - if potential_path.exists(): - font_path = potential_path - break - - # Final security check: ensure path is within fonts_dir - try: - font_path.resolve().relative_to(fonts_dir.resolve()) - except ValueError: - return jsonify({'status': 'error', 'message': 'Invalid font path'}), 400 - - if not font_path.exists(): - return jsonify({'status': 'error', 'message': f'Font file not found: {font_filename}'}), 404 - - # Parse colors - try: - bg_rgb = tuple(int(bg_color[i:i+2], 16) for i in (0, 2, 4)) - fg_rgb = tuple(int(fg_color[i:i+2], 16) for i in (0, 2, 4)) - except (ValueError, IndexError): - bg_rgb = (0, 0, 0) - fg_rgb = (255, 255, 255) - - # Load font - font = None - if str(font_path).endswith('.bdf'): - # BDF fonts require complex per-glyph rendering via freetype - # Return explicit error rather than showing misleading preview with default font - return jsonify({ - 'status': 'error', - 'message': 'BDF font preview not supported. BDF fonts will render correctly on the LED matrix.' - }), 400 - else: - # TTF/OTF fonts - try: - font = ImageFont.truetype(str(font_path), size) - except (IOError, OSError) as e: - # IOError/OSError raised for invalid/corrupt font files - logger.warning("[FontPreview] Failed to load font %s: %s", font_path, e) - font = ImageFont.load_default() - - # Calculate text size - temp_img = Image.new('RGB', (1, 1)) - temp_draw = ImageDraw.Draw(temp_img) - bbox = temp_draw.textbbox((0, 0), text, font=font) - text_width = bbox[2] - bbox[0] - text_height = bbox[3] - bbox[1] - - # Create image with padding - padding = 10 - img_width = max(text_width + padding * 2, 100) - img_height = max(text_height + padding * 2, 30) - - # Validate resulting image size to prevent memory/CPU spikes - if img_width > MAX_DIM or img_height > MAX_DIM: - return jsonify({'status': 'error', 'message': 'Requested image too large'}), 400 - if img_width * img_height > MAX_PIXELS: - return jsonify({'status': 'error', 'message': 'Requested image too large'}), 400 - - img = Image.new('RGB', (img_width, img_height), bg_rgb) - draw = ImageDraw.Draw(img) - - # Center text - x = (img_width - text_width) // 2 - y = (img_height - text_height) // 2 - - draw.text((x, y), text, font=font, fill=fg_rgb) - - # Convert to base64 - buffer = io.BytesIO() - img.save(buffer, format='PNG') - buffer.seek(0) - img_base64 = base64.b64encode(buffer.getvalue()).decode('utf-8') + # Parse colors + try: + bg_rgb = tuple(int(bg_color[i:i+2], 16) for i in (0, 2, 4)) + fg_rgb = tuple(int(fg_color[i:i+2], 16) for i in (0, 2, 4)) + except (ValueError, IndexError): + bg_rgb = (0, 0, 0) + fg_rgb = (255, 255, 255) + # Load font + font = None + if str(font_path).endswith('.bdf'): + # BDF fonts require complex per-glyph rendering via freetype + # Return explicit error rather than showing misleading preview with default font return jsonify({ - 'status': 'success', - 'data': { - 'image': f'data:image/png;base64,{img_base64}', - 'width': img_width, - 'height': img_height - } - }) - except Exception as e: - logger.error('Unhandled exception', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + 'status': 'error', + 'message': 'BDF font preview not supported. BDF fonts will render correctly on the LED matrix.' + }), 400 + else: + # TTF/OTF fonts + try: + font = ImageFont.truetype(str(font_path), size) + except (IOError, OSError) as e: + # IOError/OSError raised for invalid/corrupt font files + logger.warning("[FontPreview] Failed to load font %s: %s", font_path, e) + font = ImageFont.load_default() + + # Calculate text size + temp_img = Image.new('RGB', (1, 1)) + temp_draw = ImageDraw.Draw(temp_img) + bbox = temp_draw.textbbox((0, 0), text, font=font) + text_width = bbox[2] - bbox[0] + text_height = bbox[3] - bbox[1] + + # Create image with padding + padding = 10 + img_width = max(text_width + padding * 2, 100) + img_height = max(text_height + padding * 2, 30) + + # Validate resulting image size to prevent memory/CPU spikes + if img_width > MAX_DIM or img_height > MAX_DIM: + return jsonify({'status': 'error', 'message': 'Requested image too large'}), 400 + if img_width * img_height > MAX_PIXELS: + return jsonify({'status': 'error', 'message': 'Requested image too large'}), 400 + + img = Image.new('RGB', (img_width, img_height), bg_rgb) + draw = ImageDraw.Draw(img) + + # Center text + x = (img_width - text_width) // 2 + y = (img_height - text_height) // 2 + + draw.text((x, y), text, font=font, fill=fg_rgb) + + # Convert to base64 + buffer = io.BytesIO() + img.save(buffer, format='PNG') + buffer.seek(0) + img_base64 = base64.b64encode(buffer.getvalue()).decode('utf-8') + + return jsonify({ + 'status': 'success', + 'data': { + 'image': f'data:image/png;base64,{img_base64}', + 'width': img_width, + 'height': img_height + } + }) @api_v3.route('/fonts/', methods=['DELETE']) def delete_font(font_family: str) -> tuple[Response, int] | Response: """Delete a user-uploaded font file""" - try: - # Security: Validate font_family to prevent path traversal - # Reject if it contains path separators or .. - if '..' in font_family or '/' in font_family or '\\' in font_family: - return jsonify({'status': 'error', 'message': 'Invalid font family name'}), 400 + # Security: Validate font_family to prevent path traversal + # Reject if it contains path separators or .. + if '..' in font_family or '/' in font_family or '\\' in font_family: + return jsonify({'status': 'error', 'message': 'Invalid font family name'}), 400 - # Only allow safe characters: alphanumeric, hyphen, underscore, dot - if not re.match(r'^[a-zA-Z0-9_\-\.]+$', font_family): - return jsonify({'status': 'error', 'message': 'Invalid font family name'}), 400 + # Only allow safe characters: alphanumeric, hyphen, underscore, dot + if not re.match(r'^[a-zA-Z0-9_\-\.]+$', font_family): + return jsonify({'status': 'error', 'message': 'Invalid font family name'}), 400 - # Check if this is a system font (uses module-level SYSTEM_FONTS frozenset) - if font_family.lower() in SYSTEM_FONTS: - return jsonify({'status': 'error', 'message': 'Cannot delete system fonts'}), 403 + # Check if this is a system font (uses module-level SYSTEM_FONTS frozenset) + if font_family.lower() in SYSTEM_FONTS: + return jsonify({'status': 'error', 'message': 'Cannot delete system fonts'}), 403 - # Find and delete the font file - fonts_dir = PROJECT_ROOT / "assets" / "fonts" + # Find and delete the font file + fonts_dir = PROJECT_ROOT / "assets" / "fonts" - # Ensure fonts directory exists - if not fonts_dir.exists() or not fonts_dir.is_dir(): - return jsonify({'status': 'error', 'message': 'Fonts directory not found'}), 404 + # Ensure fonts directory exists + if not fonts_dir.exists() or not fonts_dir.is_dir(): + return jsonify({'status': 'error', 'message': 'Fonts directory not found'}), 404 - deleted = False - deleted_filename = None + deleted = False + deleted_filename = None - # Only try valid font extensions (no empty string to avoid matching directories) - for ext in ['.ttf', '.otf', '.bdf']: - potential_path = fonts_dir / f"{font_family}{ext}" + # Only try valid font extensions (no empty string to avoid matching directories) + for ext in ['.ttf', '.otf', '.bdf']: + potential_path = fonts_dir / f"{font_family}{ext}" - # Security: Verify path is within fonts_dir - try: - potential_path.resolve().relative_to(fonts_dir.resolve()) - except ValueError: - continue # Path escapes fonts_dir, skip - - if potential_path.exists() and potential_path.is_file(): - potential_path.unlink() - deleted = True - deleted_filename = f"{font_family}{ext}" - break - - if not deleted: - # Try case-insensitive match within fonts directory - font_family_lower = font_family.lower() - for filename in os.listdir(fonts_dir): - # Only consider files with valid font extensions - if not any(filename.lower().endswith(ext) for ext in ['.ttf', '.otf', '.bdf']): - continue - - name_without_ext = os.path.splitext(filename)[0] - if name_without_ext.lower() == font_family_lower: - filepath = fonts_dir / filename - - # Security: Verify path is within fonts_dir - try: - filepath.resolve().relative_to(fonts_dir.resolve()) - except ValueError: - continue # Path escapes fonts_dir, skip - - if filepath.is_file(): - filepath.unlink() - deleted = True - deleted_filename = filename - break - - if not deleted: - return jsonify({'status': 'error', 'message': f'Font not found: {font_family}'}), 404 - - # Clear font catalog cache + # Security: Verify path is within fonts_dir try: - from web_interface.cache import delete_cached - delete_cached('fonts_catalog') - except ImportError as e: - logger.warning("[FontDelete] Cache module not available: %s", e) - except Exception: - logger.error("[FontDelete] Failed to clear fonts_catalog cache", exc_info=True) + potential_path.resolve().relative_to(fonts_dir.resolve()) + except ValueError: + continue # Path escapes fonts_dir, skip - return jsonify({ - 'status': 'success', - 'message': f'Font {deleted_filename} deleted successfully' - }) - except Exception as e: - logger.error('Unhandled exception', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + if potential_path.exists() and potential_path.is_file(): + potential_path.unlink() + deleted = True + deleted_filename = f"{font_family}{ext}" + break + + if not deleted: + # Try case-insensitive match within fonts directory + font_family_lower = font_family.lower() + for filename in os.listdir(fonts_dir): + # Only consider files with valid font extensions + if not any(filename.lower().endswith(ext) for ext in ['.ttf', '.otf', '.bdf']): + continue + + name_without_ext = os.path.splitext(filename)[0] + if name_without_ext.lower() == font_family_lower: + filepath = fonts_dir / filename + + # Security: Verify path is within fonts_dir + try: + filepath.resolve().relative_to(fonts_dir.resolve()) + except ValueError: + continue # Path escapes fonts_dir, skip + + if filepath.is_file(): + filepath.unlink() + deleted = True + deleted_filename = filename + break + + if not deleted: + return jsonify({'status': 'error', 'message': f'Font not found: {font_family}'}), 404 + + # Clear font catalog cache + try: + from web_interface.cache import delete_cached + delete_cached('fonts_catalog') + except ImportError as e: + logger.warning("[FontDelete] Cache module not available: %s", e) + except Exception: + logger.error("[FontDelete] Failed to clear fonts_catalog cache", exc_info=True) + + return jsonify({ + 'status': 'success', + 'message': f'Font {deleted_filename} deleted successfully' + }) diff --git a/web_interface/blueprints/api_v3/misc.py b/web_interface/blueprints/api_v3/misc.py index 8e49b7e7..0f50f490 100644 --- a/web_interface/blueprints/api_v3/misc.py +++ b/web_interface/blueprints/api_v3/misc.py @@ -186,13 +186,6 @@ def get_logs(): 'status': 'error', 'message': 'Timeout while fetching logs' }), 500 - except Exception as e: - logger.error("%s failed", request.path, exc_info=True) - return jsonify({ - 'status': 'error', - 'message': 'An error occurred; see logs for details', - 'details': describe_exception(e) - }), 500 # Multi-Display Sync Endpoints @api_v3.route('/sync/status', methods=['GET']) def get_sync_status(): @@ -231,57 +224,49 @@ def get_sync_status(): @api_v3.route('/cache/list', methods=['GET']) def list_cache_files(): """List all cache files with metadata""" - try: - if not api_v3.cache_manager: - # Initialize cache manager if not already initialized - from src.cache_manager import CacheManager - api_v3.cache_manager = CacheManager() + if not api_v3.cache_manager: + # Initialize cache manager if not already initialized + from src.cache_manager import CacheManager + api_v3.cache_manager = CacheManager() - cache_files = api_v3.cache_manager.list_cache_files() - cache_dir = api_v3.cache_manager.get_cache_dir() + cache_files = api_v3.cache_manager.list_cache_files() + cache_dir = api_v3.cache_manager.get_cache_dir() - return jsonify({ - 'status': 'success', - 'data': { - 'cache_files': cache_files, - 'cache_dir': cache_dir, - 'total_files': len(cache_files) - } - }) - except Exception as e: - logger.error('Error in list_cache_files', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + return jsonify({ + 'status': 'success', + 'data': { + 'cache_files': cache_files, + 'cache_dir': cache_dir, + 'total_files': len(cache_files) + } + }) @api_v3.route('/cache/delete', methods=['POST']) def delete_cache_file(): """Delete a specific cache file by key""" - try: - if not api_v3.cache_manager: - # Initialize cache manager if not already initialized - from src.cache_manager import CacheManager - api_v3.cache_manager = CacheManager() + if not api_v3.cache_manager: + # Initialize cache manager if not already initialized + from src.cache_manager import CacheManager + api_v3.cache_manager = CacheManager() - data = request.get_json(silent=True) - if not data or 'key' not in data: - return jsonify({'status': 'error', 'message': 'cache key is required'}), 400 + data = request.get_json(silent=True) + if not data or 'key' not in data: + return jsonify({'status': 'error', 'message': 'cache key is required'}), 400 - cache_key = data['key'] + cache_key = data['key'] - # The key names the file about to be removed. DiskCache refuses an - # unusable key on its own, but silently: say so here instead of - # reporting a deletion that never happened. - if safe_path_component(cache_key) is None: - return jsonify({'status': 'error', 'message': 'Invalid cache key'}), 400 + # The key names the file about to be removed. DiskCache refuses an + # unusable key on its own, but silently: say so here instead of + # reporting a deletion that never happened. + if safe_path_component(cache_key) is None: + return jsonify({'status': 'error', 'message': 'Invalid cache key'}), 400 - # Delete the cache file - api_v3.cache_manager.clear_cache(cache_key) + # Delete the cache file + api_v3.cache_manager.clear_cache(cache_key) - return jsonify({ - 'status': 'success', - 'message': f'Cache file for key "{cache_key}" deleted successfully' - }) - except Exception as e: - logger.error('Error in delete_cache_file', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + return jsonify({ + 'status': 'success', + 'message': f'Cache file for key "{cache_key}" deleted successfully' + }) def _errors_cache(): """The shared cache the display service publishes its errors to.""" if not api_v3.cache_manager: diff --git a/web_interface/blueprints/api_v3/plugins.py b/web_interface/blueprints/api_v3/plugins.py index b293d321..24ecc362 100644 --- a/web_interface/blueprints/api_v3/plugins.py +++ b/web_interface/blueprints/api_v3/plugins.py @@ -14,6 +14,7 @@ from web_interface.blueprints.api_v3 import ( _set_missing_booleans_to_false, _set_nested_value, _starlark_virtual_plugins, _toggle_starlark_app, api_v3, datetime, deep_merge, describe_exception, error_response, + exception_error_response, find_secret_fields, hashlib, json, jsonify, logger, logging, merge_secrets, os, redact_text, remove_empty_secrets, request, separate_secrets, shutil, stat, subprocess, success_response, @@ -32,380 +33,348 @@ import web_interface.blueprints.api_v3 as _pkg @api_v3.route('/plugins/installed', methods=['GET']) def get_installed_plugins(): """Get installed plugins""" - try: - if not api_v3.plugin_manager or not api_v3.plugin_store_manager: - return jsonify({'status': 'error', 'message': 'Plugin managers not initialized'}), 500 + if not api_v3.plugin_manager or not api_v3.plugin_store_manager: + return jsonify({'status': 'error', 'message': 'Plugin managers not initialized'}), 500 - import json - from pathlib import Path + import json + from pathlib import Path - # Re-discover plugins to ensure we have the latest list - # This handles cases where plugins are added/removed after app startup - api_v3.plugin_manager.discover_plugins() + # Re-discover plugins to ensure we have the latest list + # This handles cases where plugins are added/removed after app startup + api_v3.plugin_manager.discover_plugins() - # Get all installed plugin info from the plugin manager - all_plugin_info = api_v3.plugin_manager.get_all_plugin_info() + # Get all installed plugin info from the plugin manager + all_plugin_info = api_v3.plugin_manager.get_all_plugin_info() - # Load config once before the loop (not per-plugin) - full_config = api_v3.config_manager.load_config() if api_v3.config_manager else {} + # Load config once before the loop (not per-plugin) + full_config = api_v3.config_manager.load_config() if api_v3.config_manager else {} - def _build_plugin_entry(plugin_info): - plugin_id = plugin_info.get('id') + def _build_plugin_entry(plugin_info): + plugin_id = plugin_info.get('id') + try: + return _build_plugin_entry_inner(plugin_info, plugin_id) + except Exception: + logger.exception("Error building plugin entry for %s — skipping", plugin_id) + return None + + def _build_plugin_entry_inner(plugin_info, plugin_id): + # Capture runtime state (state machine + error context) before the + # manifest merge below can shadow the 'state' key. get_all_plugin_info + # attaches this via PluginStateManager.get_state_info(); surfacing it + # lets the UI show *why* a plugin isn't running instead of just + # 'loaded: false'. + state_info = plugin_info.get('state') + plugin_state = None + plugin_error_info = None + if isinstance(state_info, dict): + plugin_state = state_info.get('state') + plugin_error_info = state_info.get('error_info') + + # Re-read manifest from disk to ensure we have the latest metadata + manifest_path = Path(api_v3.plugin_manager.plugins_dir) / plugin_id / "manifest.json" + if manifest_path.exists(): try: - return _build_plugin_entry_inner(plugin_info, plugin_id) - except Exception: - logger.exception("Error building plugin entry for %s — skipping", plugin_id) - return None + with open(manifest_path, 'r', encoding='utf-8') as f: + fresh_manifest = json.load(f) + if isinstance(fresh_manifest, dict): + plugin_info.update(fresh_manifest) + else: + logger.debug("Manifest for %s is not a dict (%s) — skipping merge", + plugin_id, type(fresh_manifest).__name__) + except (FileNotFoundError, PermissionError, json.JSONDecodeError) as e: + logger.debug("Could not read fresh manifest for %s: %s", plugin_id, e) - def _build_plugin_entry_inner(plugin_info, plugin_id): - # Capture runtime state (state machine + error context) before the - # manifest merge below can shadow the 'state' key. get_all_plugin_info - # attaches this via PluginStateManager.get_state_info(); surfacing it - # lets the UI show *why* a plugin isn't running instead of just - # 'loaded: false'. - state_info = plugin_info.get('state') - plugin_state = None - plugin_error_info = None - if isinstance(state_info, dict): - plugin_state = state_info.get('state') - plugin_error_info = state_info.get('error_info') + # Enabled status: config is source of truth, fall back to instance + enabled = None + plugin_config = full_config.get(plugin_id, {}) + if 'enabled' in plugin_config: + enabled = bool(plugin_config['enabled']) - # Re-read manifest from disk to ensure we have the latest metadata - manifest_path = Path(api_v3.plugin_manager.plugins_dir) / plugin_id / "manifest.json" - if manifest_path.exists(): - try: - with open(manifest_path, 'r', encoding='utf-8') as f: - fresh_manifest = json.load(f) - if isinstance(fresh_manifest, dict): - plugin_info.update(fresh_manifest) - else: - logger.debug("Manifest for %s is not a dict (%s) — skipping merge", - plugin_id, type(fresh_manifest).__name__) - except (FileNotFoundError, PermissionError, json.JSONDecodeError) as e: - logger.debug("Could not read fresh manifest for %s: %s", plugin_id, e) + # Single get_plugin() call shared for both enabled fallback and Vegas mode + plugin_instance = api_v3.plugin_manager.get_plugin(plugin_id) + if enabled is None: + enabled = plugin_instance.enabled if plugin_instance else True - # Enabled status: config is source of truth, fall back to instance - enabled = None - plugin_config = full_config.get(plugin_id, {}) - if 'enabled' in plugin_config: - enabled = bool(plugin_config['enabled']) + # Verified + latest published version from registry (no network call) + store_info = api_v3.plugin_store_manager.get_registry_info(plugin_id) + verified = store_info.get('verified', False) if store_info else False + latest_version = store_info.get('latest_version', '') if store_info else '' + installed_version = plugin_info.get('version', '') + update_available = _is_plugin_update_available(installed_version, latest_version) - # Single get_plugin() call shared for both enabled fallback and Vegas mode - plugin_instance = api_v3.plugin_manager.get_plugin(plugin_id) - if enabled is None: - enabled = plugin_instance.enabled if plugin_instance else True + # Local git info (single subprocess on cache miss, zero on hit) + plugin_path = Path(api_v3.plugin_manager.plugins_dir) / plugin_id + local_git_info = api_v3.plugin_store_manager._get_local_git_info(plugin_path) if plugin_path.exists() else None - # Verified + latest published version from registry (no network call) - store_info = api_v3.plugin_store_manager.get_registry_info(plugin_id) - verified = store_info.get('verified', False) if store_info else False - latest_version = store_info.get('latest_version', '') if store_info else '' - installed_version = plugin_info.get('version', '') - update_available = _is_plugin_update_available(installed_version, latest_version) + if local_git_info: + sha = local_git_info.get('sha', '') + last_commit = local_git_info.get('short_sha') or (sha[:7] if sha else None) + branch = local_git_info.get('branch') + last_updated = local_git_info.get('date_iso') or local_git_info.get('date') + else: + last_updated = plugin_info.get('last_updated') + last_commit = plugin_info.get('last_commit') or plugin_info.get('last_commit_sha') + branch = plugin_info.get('branch') + if store_info: + last_updated = last_updated or store_info.get('last_updated') or store_info.get('last_updated_iso') + last_commit = last_commit or store_info.get('last_commit') or store_info.get('last_commit_sha') + branch = branch or store_info.get('branch') or store_info.get('default_branch') - # Local git info (single subprocess on cache miss, zero on hit) - plugin_path = Path(api_v3.plugin_manager.plugins_dir) / plugin_id - local_git_info = api_v3.plugin_store_manager._get_local_git_info(plugin_path) if plugin_path.exists() else None + last_commit_message = plugin_info.get('last_commit_message') + if store_info and not last_commit_message: + last_commit_message = store_info.get('last_commit_message') - if local_git_info: - sha = local_git_info.get('sha', '') - last_commit = local_git_info.get('short_sha') or (sha[:7] if sha else None) - branch = local_git_info.get('branch') - last_updated = local_git_info.get('date_iso') or local_git_info.get('date') - else: - last_updated = plugin_info.get('last_updated') - last_commit = plugin_info.get('last_commit') or plugin_info.get('last_commit_sha') - branch = plugin_info.get('branch') - if store_info: - last_updated = last_updated or store_info.get('last_updated') or store_info.get('last_updated_iso') - last_commit = last_commit or store_info.get('last_commit') or store_info.get('last_commit_sha') - branch = branch or store_info.get('branch') or store_info.get('default_branch') + # Vegas mode from instance, overridden by explicit config value + vegas_mode = None + vegas_content_type = None + if plugin_instance: + try: + if hasattr(plugin_instance, 'get_vegas_display_mode'): + mode = plugin_instance.get_vegas_display_mode() + vegas_mode = mode.value if hasattr(mode, 'value') else str(mode) + except (AttributeError, TypeError, ValueError) as e: + logger.debug("[%s] Failed to get vegas_display_mode: %s", plugin_id, e) + try: + if hasattr(plugin_instance, 'get_vegas_content_type'): + vegas_content_type = plugin_instance.get_vegas_content_type() + except (AttributeError, TypeError, ValueError) as e: + logger.debug("[%s] Failed to get vegas_content_type: %s", plugin_id, e) - last_commit_message = plugin_info.get('last_commit_message') - if store_info and not last_commit_message: - last_commit_message = store_info.get('last_commit_message') + if 'vegas_mode' in plugin_config: + vegas_mode = plugin_config['vegas_mode'] - # Vegas mode from instance, overridden by explicit config value - vegas_mode = None - vegas_content_type = None - if plugin_instance: - try: - if hasattr(plugin_instance, 'get_vegas_display_mode'): - mode = plugin_instance.get_vegas_display_mode() - vegas_mode = mode.value if hasattr(mode, 'value') else str(mode) - except (AttributeError, TypeError, ValueError) as e: - logger.debug("[%s] Failed to get vegas_display_mode: %s", plugin_id, e) - try: - if hasattr(plugin_instance, 'get_vegas_content_type'): - vegas_content_type = plugin_instance.get_vegas_content_type() - except (AttributeError, TypeError, ValueError) as e: - logger.debug("[%s] Failed to get vegas_content_type: %s", plugin_id, e) + return { + 'id': plugin_id, + 'name': plugin_info.get('name', plugin_id), + 'version': plugin_info.get('version', ''), + 'latest_version': latest_version, + 'update_available': update_available, + 'author': plugin_info.get('author', 'Unknown'), + 'category': plugin_info.get('category', 'General'), + 'description': plugin_info.get('description', 'No description available'), + 'tags': plugin_info.get('tags', []), + 'enabled': enabled, + 'verified': verified, + 'loaded': plugin_info.get('loaded', False), + 'state': plugin_state, + 'error_info': plugin_error_info, + 'last_updated': last_updated, + 'last_commit': last_commit, + 'last_commit_message': last_commit_message, + 'branch': branch, + 'web_ui_actions': plugin_info.get('web_ui_actions', []), + 'vegas_mode': vegas_mode, + 'vegas_content_type': vegas_content_type, + } - if 'vegas_mode' in plugin_config: - vegas_mode = plugin_config['vegas_mode'] + from concurrent.futures import ThreadPoolExecutor + with ThreadPoolExecutor(max_workers=8) as executor: + results = list(executor.map(_build_plugin_entry, all_plugin_info)) + plugins = [r for r in results if r is not None] + plugins.extend(_starlark_virtual_plugins()) - return { - 'id': plugin_id, - 'name': plugin_info.get('name', plugin_id), - 'version': plugin_info.get('version', ''), - 'latest_version': latest_version, - 'update_available': update_available, - 'author': plugin_info.get('author', 'Unknown'), - 'category': plugin_info.get('category', 'General'), - 'description': plugin_info.get('description', 'No description available'), - 'tags': plugin_info.get('tags', []), - 'enabled': enabled, - 'verified': verified, - 'loaded': plugin_info.get('loaded', False), - 'state': plugin_state, - 'error_info': plugin_error_info, - 'last_updated': last_updated, - 'last_commit': last_commit, - 'last_commit_message': last_commit_message, - 'branch': branch, - 'web_ui_actions': plugin_info.get('web_ui_actions', []), - 'vegas_mode': vegas_mode, - 'vegas_content_type': vegas_content_type, - } - - from concurrent.futures import ThreadPoolExecutor - with ThreadPoolExecutor(max_workers=8) as executor: - results = list(executor.map(_build_plugin_entry, all_plugin_info)) - plugins = [r for r in results if r is not None] - plugins.extend(_starlark_virtual_plugins()) - - return jsonify({'status': 'success', 'data': {'plugins': plugins}}) - except Exception as e: - logger.error('Error in get_installed_plugins', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + return jsonify({'status': 'success', 'data': {'plugins': plugins}}) @api_v3.route('/plugins/health', methods=['GET']) def get_plugin_health(): """Get health metrics for all plugins""" - try: - if not api_v3.plugin_manager: - return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 - - # Check if health tracker is available - if not hasattr(api_v3.plugin_manager, 'health_tracker') or not api_v3.plugin_manager.health_tracker: - return jsonify({ - 'status': 'success', - 'data': {}, - 'message': 'Health tracking not available' - }) - - tracker = api_v3.plugin_manager.health_tracker - # Build per-plugin summaries by ID so persisted (cross-process) health - # is included, then fold in any in-memory-only entries. - health_summaries = {} - for pid in _installed_plugin_ids(): - try: - # force_reload: this process only reads; bypass the in-memory - # snapshot so each poll reflects the display service's latest - # persisted state. - health_summaries[pid] = tracker.get_health_summary(pid, force_reload=True) - except Exception: - logger.debug('Could not read health summary for %s', pid, exc_info=True) - try: - for pid, summary in tracker.get_all_health_summaries().items(): - health_summaries.setdefault(pid, summary) - except Exception: - logger.debug('get_all_health_summaries failed', exc_info=True) + if not api_v3.plugin_manager: + return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 + # Check if health tracker is available + if not hasattr(api_v3.plugin_manager, 'health_tracker') or not api_v3.plugin_manager.health_tracker: return jsonify({ 'status': 'success', - 'data': health_summaries + 'data': {}, + 'message': 'Health tracking not available' }) - except Exception as e: - logger.error('Error in get_plugin_health', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + + tracker = api_v3.plugin_manager.health_tracker + # Build per-plugin summaries by ID so persisted (cross-process) health + # is included, then fold in any in-memory-only entries. + health_summaries = {} + for pid in _installed_plugin_ids(): + try: + # force_reload: this process only reads; bypass the in-memory + # snapshot so each poll reflects the display service's latest + # persisted state. + health_summaries[pid] = tracker.get_health_summary(pid, force_reload=True) + except Exception: + logger.debug('Could not read health summary for %s', pid, exc_info=True) + try: + for pid, summary in tracker.get_all_health_summaries().items(): + health_summaries.setdefault(pid, summary) + except Exception: + logger.debug('get_all_health_summaries failed', exc_info=True) + + return jsonify({ + 'status': 'success', + 'data': health_summaries + }) @api_v3.route('/plugins/health/', methods=['GET']) def get_plugin_health_single(plugin_id): """Get health metrics for a specific plugin""" - try: - if not api_v3.plugin_manager: - return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 - - # Check if health tracker is available - if not hasattr(api_v3.plugin_manager, 'health_tracker') or not api_v3.plugin_manager.health_tracker: - return jsonify({ - 'status': 'error', - 'message': 'Health tracking not available' - }), 503 - - # Get health summary for specific plugin - health_summary = api_v3.plugin_manager.health_tracker.get_health_summary(plugin_id) + if not api_v3.plugin_manager: + return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 + # Check if health tracker is available + if not hasattr(api_v3.plugin_manager, 'health_tracker') or not api_v3.plugin_manager.health_tracker: return jsonify({ - 'status': 'success', - 'data': health_summary - }) - except Exception as e: - logger.error('Error in get_plugin_health_single', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + 'status': 'error', + 'message': 'Health tracking not available' + }), 503 + + # Get health summary for specific plugin + health_summary = api_v3.plugin_manager.health_tracker.get_health_summary(plugin_id) + + return jsonify({ + 'status': 'success', + 'data': health_summary + }) @api_v3.route('/plugins/health//reset', methods=['POST']) def reset_plugin_health(plugin_id): """Reset health state for a plugin (manual recovery)""" - try: - if not api_v3.plugin_manager: - return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 - - # Check if health tracker is available - if not hasattr(api_v3.plugin_manager, 'health_tracker') or not api_v3.plugin_manager.health_tracker: - return jsonify({ - 'status': 'error', - 'message': 'Health tracking not available' - }), 503 - - # Reset health state - api_v3.plugin_manager.health_tracker.reset_health(plugin_id) + if not api_v3.plugin_manager: + return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 + # Check if health tracker is available + if not hasattr(api_v3.plugin_manager, 'health_tracker') or not api_v3.plugin_manager.health_tracker: return jsonify({ - 'status': 'success', - 'message': f'Health state reset for plugin {plugin_id}' - }) - except Exception as e: - logger.error('Error in reset_plugin_health', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + 'status': 'error', + 'message': 'Health tracking not available' + }), 503 + + # Reset health state + api_v3.plugin_manager.health_tracker.reset_health(plugin_id) + + return jsonify({ + 'status': 'success', + 'message': f'Health state reset for plugin {plugin_id}' + }) @api_v3.route('/plugins/metrics', methods=['GET']) def get_plugin_metrics(): """Get resource metrics for all plugins""" - try: - if not api_v3.plugin_manager: - return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 - - # Check if resource monitor is available - if not hasattr(api_v3.plugin_manager, 'resource_monitor') or not api_v3.plugin_manager.resource_monitor: - return jsonify({ - 'status': 'success', - 'data': {}, - 'message': 'Resource monitoring not available' - }) - - monitor = api_v3.plugin_manager.resource_monitor - # Build per-plugin summaries by ID so persisted (cross-process) metrics - # are included, then fold in any in-memory-only entries. - metrics_summaries = {} - for pid in _installed_plugin_ids(): - try: - # force_reload: read-only path — bypass the in-memory snapshot so - # each poll reflects the display service's latest persisted metrics. - metrics_summaries[pid] = monitor.get_metrics_summary(pid, force_reload=True) - except Exception: - logger.debug('Could not read metrics summary for %s', pid, exc_info=True) - try: - for pid, summary in monitor.get_all_metrics_summaries().items(): - metrics_summaries.setdefault(pid, summary) - except Exception: - logger.debug('get_all_metrics_summaries failed', exc_info=True) + if not api_v3.plugin_manager: + return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 + # Check if resource monitor is available + if not hasattr(api_v3.plugin_manager, 'resource_monitor') or not api_v3.plugin_manager.resource_monitor: return jsonify({ 'status': 'success', - 'data': metrics_summaries + 'data': {}, + 'message': 'Resource monitoring not available' }) - except Exception as e: - logger.error('Error in get_plugin_metrics', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + + monitor = api_v3.plugin_manager.resource_monitor + # Build per-plugin summaries by ID so persisted (cross-process) metrics + # are included, then fold in any in-memory-only entries. + metrics_summaries = {} + for pid in _installed_plugin_ids(): + try: + # force_reload: read-only path — bypass the in-memory snapshot so + # each poll reflects the display service's latest persisted metrics. + metrics_summaries[pid] = monitor.get_metrics_summary(pid, force_reload=True) + except Exception: + logger.debug('Could not read metrics summary for %s', pid, exc_info=True) + try: + for pid, summary in monitor.get_all_metrics_summaries().items(): + metrics_summaries.setdefault(pid, summary) + except Exception: + logger.debug('get_all_metrics_summaries failed', exc_info=True) + + return jsonify({ + 'status': 'success', + 'data': metrics_summaries + }) @api_v3.route('/plugins/metrics/', methods=['GET']) def get_plugin_metrics_single(plugin_id): """Get resource metrics for a specific plugin""" - try: - if not api_v3.plugin_manager: - return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 - - # Check if resource monitor is available - if not hasattr(api_v3.plugin_manager, 'resource_monitor') or not api_v3.plugin_manager.resource_monitor: - return jsonify({ - 'status': 'error', - 'message': 'Resource monitoring not available' - }), 503 - - # Get metrics summary for specific plugin - metrics_summary = api_v3.plugin_manager.resource_monitor.get_metrics_summary(plugin_id) + if not api_v3.plugin_manager: + return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 + # Check if resource monitor is available + if not hasattr(api_v3.plugin_manager, 'resource_monitor') or not api_v3.plugin_manager.resource_monitor: return jsonify({ - 'status': 'success', - 'data': metrics_summary - }) - except Exception as e: - logger.error('Error in get_plugin_metrics_single', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + 'status': 'error', + 'message': 'Resource monitoring not available' + }), 503 + + # Get metrics summary for specific plugin + metrics_summary = api_v3.plugin_manager.resource_monitor.get_metrics_summary(plugin_id) + + return jsonify({ + 'status': 'success', + 'data': metrics_summary + }) @api_v3.route('/plugins/metrics//reset', methods=['POST']) def reset_plugin_metrics(plugin_id): """Reset metrics for a plugin""" - try: - if not api_v3.plugin_manager: - return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 - - # Check if resource monitor is available - if not hasattr(api_v3.plugin_manager, 'resource_monitor') or not api_v3.plugin_manager.resource_monitor: - return jsonify({ - 'status': 'error', - 'message': 'Resource monitoring not available' - }), 503 - - # Reset metrics - api_v3.plugin_manager.resource_monitor.reset_metrics(plugin_id) + if not api_v3.plugin_manager: + return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 + # Check if resource monitor is available + if not hasattr(api_v3.plugin_manager, 'resource_monitor') or not api_v3.plugin_manager.resource_monitor: return jsonify({ - 'status': 'success', - 'message': f'Metrics reset for plugin {plugin_id}' - }) - except Exception as e: - logger.error('Error in reset_plugin_metrics', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + 'status': 'error', + 'message': 'Resource monitoring not available' + }), 503 + + # Reset metrics + api_v3.plugin_manager.resource_monitor.reset_metrics(plugin_id) + + return jsonify({ + 'status': 'success', + 'message': f'Metrics reset for plugin {plugin_id}' + }) @api_v3.route('/plugins/limits/', methods=['GET', 'POST']) def manage_plugin_limits(plugin_id): """Get or set resource limits for a plugin""" - try: - if not api_v3.plugin_manager: - return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 + if not api_v3.plugin_manager: + return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 - # Check if resource monitor is available - if not hasattr(api_v3.plugin_manager, 'resource_monitor') or not api_v3.plugin_manager.resource_monitor: - return jsonify({ - 'status': 'error', - 'message': 'Resource monitoring not available' - }), 503 - - if request.method == 'GET': - # Get limits - limits = api_v3.plugin_manager.resource_monitor.get_limits(plugin_id) - if limits: - return jsonify({ - 'status': 'success', - 'data': { - 'max_memory_mb': limits.max_memory_mb, - 'max_cpu_percent': limits.max_cpu_percent, - 'max_execution_time': limits.max_execution_time, - 'warning_threshold': limits.warning_threshold - } - }) - else: - return jsonify({ - 'status': 'success', - 'data': None, - 'message': 'No limits configured for this plugin' - }) - else: - # POST - Set limits - data = request.get_json(silent=True) or {} - from src.plugin_system.resource_monitor import ResourceLimits - - limits = ResourceLimits( - max_memory_mb=data.get('max_memory_mb'), - max_cpu_percent=data.get('max_cpu_percent'), - max_execution_time=data.get('max_execution_time'), - warning_threshold=data.get('warning_threshold', 0.8) - ) - - api_v3.plugin_manager.resource_monitor.set_limits(plugin_id, limits) + # Check if resource monitor is available + if not hasattr(api_v3.plugin_manager, 'resource_monitor') or not api_v3.plugin_manager.resource_monitor: + return jsonify({ + 'status': 'error', + 'message': 'Resource monitoring not available' + }), 503 + if request.method == 'GET': + # Get limits + limits = api_v3.plugin_manager.resource_monitor.get_limits(plugin_id) + if limits: return jsonify({ 'status': 'success', - 'message': f'Resource limits updated for plugin {plugin_id}' + 'data': { + 'max_memory_mb': limits.max_memory_mb, + 'max_cpu_percent': limits.max_cpu_percent, + 'max_execution_time': limits.max_execution_time, + 'warning_threshold': limits.warning_threshold + } }) - except Exception as e: - logger.error('Error in manage_plugin_limits', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + else: + return jsonify({ + 'status': 'success', + 'data': None, + 'message': 'No limits configured for this plugin' + }) + else: + # POST - Set limits + data = request.get_json(silent=True) or {} + from src.plugin_system.resource_monitor import ResourceLimits + + limits = ResourceLimits( + max_memory_mb=data.get('max_memory_mb'), + max_cpu_percent=data.get('max_cpu_percent'), + max_execution_time=data.get('max_execution_time'), + warning_threshold=data.get('warning_threshold', 0.8) + ) + + api_v3.plugin_manager.resource_monitor.set_limits(plugin_id, limits) + + return jsonify({ + 'status': 'success', + 'message': f'Resource limits updated for plugin {plugin_id}' + }) @api_v3.route('/plugins/toggle', methods=['POST']) def toggle_plugin(): """Toggle plugin enabled/disabled""" @@ -546,14 +515,7 @@ def get_operation_status(operation_id): return success_response(data=operation.to_dict()) except Exception as e: - from src.web_interface.errors import WebInterfaceError - error = WebInterfaceError.from_exception(e, ErrorCode.SYSTEM_ERROR) - return error_response( - error.error_code, - error.message, - details=error.details, - status_code=500 - ) + return exception_error_response(e, ErrorCode.SYSTEM_ERROR, with_context=False) @api_v3.route('/plugins/operation/history', methods=['GET']) def get_operation_history() -> Response: """Get operation history from the audit log.""" @@ -578,9 +540,7 @@ def get_operation_history() -> Response: operation_type=operation_type ) except (AttributeError, RuntimeError) as e: - from src.web_interface.errors import WebInterfaceError - error = WebInterfaceError.from_exception(e, ErrorCode.SYSTEM_ERROR) - return error_response(error.error_code, error.message, details=error.details, status_code=500) + return exception_error_response(e, ErrorCode.SYSTEM_ERROR, with_context=False) return success_response(data=[record.to_dict() for record in history]) @api_v3.route('/plugins/operation/history', methods=['DELETE']) @@ -596,9 +556,7 @@ def clear_operation_history() -> Response: try: api_v3.operation_history.clear_history() except (OSError, RuntimeError) as e: - from src.web_interface.errors import WebInterfaceError - error = WebInterfaceError.from_exception(e, ErrorCode.SYSTEM_ERROR) - return error_response(error.error_code, error.message, details=error.details, status_code=500) + return exception_error_response(e, ErrorCode.SYSTEM_ERROR, with_context=False) return success_response(message='Operation history cleared') @api_v3.route('/plugins/state', methods=['GET']) @@ -633,15 +591,7 @@ def get_plugin_state(): for plugin_id, state in all_states.items() }) except Exception as e: - from src.web_interface.errors import WebInterfaceError - error = WebInterfaceError.from_exception(e, ErrorCode.SYSTEM_ERROR) - return error_response( - error.error_code, - error.message, - details=error.details, - context=error.context, - status_code=500 - ) + return exception_error_response(e, ErrorCode.SYSTEM_ERROR) @api_v3.route('/plugins/state/reconcile', methods=['POST']) def reconcile_plugin_state(): """Reconcile plugin state across all sources""" @@ -705,15 +655,7 @@ def reconcile_plugin_state(): message=result.message ) except Exception as e: - from src.web_interface.errors import WebInterfaceError - error = WebInterfaceError.from_exception(e, ErrorCode.SYSTEM_ERROR) - return error_response( - error.error_code, - error.message, - details=error.details, - context=error.context, - status_code=500 - ) + return exception_error_response(e, ErrorCode.SYSTEM_ERROR) def _drop_stale_reconciliation_findings(unresolved): """Re-check a stored reconciliation verdict against current state. @@ -915,15 +857,7 @@ def get_plugin_config(): return success_response(data=plugin_config) except Exception as e: - from src.web_interface.errors import WebInterfaceError - error = WebInterfaceError.from_exception(e, ErrorCode.CONFIG_LOAD_FAILED) - return error_response( - error.error_code, - error.message, - details=error.details, - context=error.context, - status_code=500 - ) + return exception_error_response(e, ErrorCode.CONFIG_LOAD_FAILED) @api_v3.route('/plugins/update', methods=['POST']) def update_plugin(): """Update plugin""" @@ -1168,8 +1102,6 @@ def update_plugin(): except Exception as e: logger.error("Unhandled exception in update endpoint", exc_info=True) - from src.web_interface.errors import WebInterfaceError - error = WebInterfaceError.from_exception(e, ErrorCode.PLUGIN_UPDATE_FAILED) if api_v3.operation_history: api_v3.operation_history.record_operation( "update", @@ -1177,13 +1109,7 @@ def update_plugin(): status="failed", error=str(e) ) - return error_response( - error.error_code, - error.message, - details=error.details, - context=error.context, - status_code=500 - ) + return exception_error_response(e, ErrorCode.PLUGIN_UPDATE_FAILED) @api_v3.route('/plugins/uninstall', methods=['POST']) def uninstall_plugin(): """Uninstall plugin""" @@ -1266,8 +1192,6 @@ def uninstall_plugin(): ) except Exception as e: - from src.web_interface.errors import WebInterfaceError - error = WebInterfaceError.from_exception(e, ErrorCode.PLUGIN_UNINSTALL_FAILED) if api_v3.operation_history: api_v3.operation_history.record_operation( "uninstall", @@ -1275,120 +1199,58 @@ def uninstall_plugin(): status="failed", error=str(e) ) - return error_response( - error.error_code, - error.message, - details=error.details, - context=error.context, - status_code=500 - ) + return exception_error_response(e, ErrorCode.PLUGIN_UNINSTALL_FAILED) @api_v3.route('/plugins/install', methods=['POST']) def install_plugin(): """Install plugin from store""" + if not api_v3.plugin_store_manager: + return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500 + + data = request.get_json(silent=True) + if not data or 'plugin_id' not in data: + return jsonify({'status': 'error', 'message': 'plugin_id required'}), 400 + + plugin_id = data['plugin_id'] + branch = data.get('branch') # Optional branch parameter + + # A registry entry that isn't a plugin (a custom registry can still + # list old "type": "skin" entries) gets a clear refusal, not a failed + # install. try: - if not api_v3.plugin_store_manager: - return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500 + registry_entry = api_v3.plugin_store_manager.get_registry_info(plugin_id) + except Exception: + registry_entry = None + if isinstance(registry_entry, dict) and not api_v3.plugin_store_manager.is_plugin_entry(registry_entry): + return jsonify({'status': 'error', + 'message': f"{plugin_id} is a {registry_entry.get('type')!r} entry, not a plugin"}), 400 - data = request.get_json(silent=True) - if not data or 'plugin_id' not in data: - return jsonify({'status': 'error', 'message': 'plugin_id required'}), 400 + # Install the plugin + # Log the plugins directory being used for debugging + plugins_dir = api_v3.plugin_store_manager.plugins_dir + branch_info = f" (branch: {branch})" if branch else "" + logger.info("Installing plugin to directory: %s", plugins_dir) - plugin_id = data['plugin_id'] - branch = data.get('branch') # Optional branch parameter - - # A registry entry that isn't a plugin (a custom registry can still - # list old "type": "skin" entries) gets a clear refusal, not a failed - # install. - try: - registry_entry = api_v3.plugin_store_manager.get_registry_info(plugin_id) - except Exception: - registry_entry = None - if isinstance(registry_entry, dict) and not api_v3.plugin_store_manager.is_plugin_entry(registry_entry): - return jsonify({'status': 'error', - 'message': f"{plugin_id} is a {registry_entry.get('type')!r} entry, not a plugin"}), 400 - - # Install the plugin - # Log the plugins directory being used for debugging - plugins_dir = api_v3.plugin_store_manager.plugins_dir - branch_info = f" (branch: {branch})" if branch else "" - logger.info("Installing plugin to directory: %s", plugins_dir) - - # Use operation queue if available - if api_v3.operation_queue: - def install_callback(operation): - """Callback to execute plugin installation.""" - success = api_v3.plugin_store_manager.install_plugin(plugin_id, branch=branch) - - if success: - # Invalidate schema cache - if api_v3.schema_manager: - api_v3.schema_manager.invalidate_cache(plugin_id) - - # Discover and load the new plugin - if api_v3.plugin_manager: - api_v3.plugin_manager.discover_plugins() - api_v3.plugin_manager.load_plugin(plugin_id) - - # Update state manager - if api_v3.plugin_state_manager: - api_v3.plugin_state_manager.set_plugin_installed(plugin_id) - - # Record in history - if api_v3.operation_history: - version = _get_plugin_version(plugin_id) - api_v3.operation_history.record_operation( - "install", - plugin_id=plugin_id, - status="success", - details={"version": version, "branch": branch} - ) - - branch_msg = f" (branch: {branch})" if branch else "" - return {'success': True, 'message': f'Plugin {plugin_id} installed successfully{branch_msg}'} - else: - error_msg = f'Failed to install plugin {plugin_id}' - if branch: - error_msg += f' (branch: {branch})' - plugin_info = api_v3.plugin_store_manager.get_plugin_info(plugin_id) - if not plugin_info: - error_msg += ' (plugin not found in registry)' - - # Record failure in history - if api_v3.operation_history: - api_v3.operation_history.record_operation( - "install", - plugin_id=plugin_id, - status="failed", - error=error_msg, - details={"branch": branch} - ) - - raise Exception(error_msg) - - # Enqueue operation - operation_id = api_v3.operation_queue.enqueue_operation( - OperationType.INSTALL, - plugin_id, - operation_callback=install_callback - ) - - branch_msg = f" (branch: {branch})" if branch else "" - return success_response( - data={'operation_id': operation_id}, - message=f'Plugin {plugin_id} installation queued{branch_msg}' - ) - else: - # Fallback to direct installation + # Use operation queue if available + if api_v3.operation_queue: + def install_callback(operation): + """Callback to execute plugin installation.""" success = api_v3.plugin_store_manager.install_plugin(plugin_id, branch=branch) if success: + # Invalidate schema cache if api_v3.schema_manager: api_v3.schema_manager.invalidate_cache(plugin_id) + + # Discover and load the new plugin if api_v3.plugin_manager: api_v3.plugin_manager.discover_plugins() api_v3.plugin_manager.load_plugin(plugin_id) + + # Update state manager if api_v3.plugin_state_manager: api_v3.plugin_state_manager.set_plugin_installed(plugin_id) + + # Record in history if api_v3.operation_history: version = _get_plugin_version(plugin_id) api_v3.operation_history.record_operation( @@ -1399,7 +1261,7 @@ def install_plugin(): ) branch_msg = f" (branch: {branch})" if branch else "" - return success_response(message=f'Plugin installed successfully{branch_msg}') + return {'success': True, 'message': f'Plugin {plugin_id} installed successfully{branch_msg}'} else: error_msg = f'Failed to install plugin {plugin_id}' if branch: @@ -1408,6 +1270,7 @@ def install_plugin(): if not plugin_info: error_msg += ' (plugin not found in registry)' + # Record failure in history if api_v3.operation_history: api_v3.operation_history.record_operation( "install", @@ -1417,317 +1280,336 @@ def install_plugin(): details={"branch": branch} ) - return error_response( - ErrorCode.PLUGIN_INSTALL_FAILED, - error_msg, - status_code=500 + raise Exception(error_msg) + + # Enqueue operation + operation_id = api_v3.operation_queue.enqueue_operation( + OperationType.INSTALL, + plugin_id, + operation_callback=install_callback + ) + + branch_msg = f" (branch: {branch})" if branch else "" + return success_response( + data={'operation_id': operation_id}, + message=f'Plugin {plugin_id} installation queued{branch_msg}' + ) + else: + # Fallback to direct installation + success = api_v3.plugin_store_manager.install_plugin(plugin_id, branch=branch) + + if success: + if api_v3.schema_manager: + api_v3.schema_manager.invalidate_cache(plugin_id) + if api_v3.plugin_manager: + api_v3.plugin_manager.discover_plugins() + api_v3.plugin_manager.load_plugin(plugin_id) + if api_v3.plugin_state_manager: + api_v3.plugin_state_manager.set_plugin_installed(plugin_id) + if api_v3.operation_history: + version = _get_plugin_version(plugin_id) + api_v3.operation_history.record_operation( + "install", + plugin_id=plugin_id, + status="success", + details={"version": version, "branch": branch} ) - except Exception as e: - logger.error('Error in install_plugin', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + branch_msg = f" (branch: {branch})" if branch else "" + return success_response(message=f'Plugin installed successfully{branch_msg}') + else: + error_msg = f'Failed to install plugin {plugin_id}' + if branch: + error_msg += f' (branch: {branch})' + plugin_info = api_v3.plugin_store_manager.get_plugin_info(plugin_id) + if not plugin_info: + error_msg += ' (plugin not found in registry)' + + if api_v3.operation_history: + api_v3.operation_history.record_operation( + "install", + plugin_id=plugin_id, + status="failed", + error=error_msg, + details={"branch": branch} + ) + + return error_response( + ErrorCode.PLUGIN_INSTALL_FAILED, + error_msg, + status_code=500 + ) + @api_v3.route('/plugins/install-from-url', methods=['POST']) def install_plugin_from_url(): """Install plugin from custom GitHub URL""" - try: - if not api_v3.plugin_store_manager: - return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500 + if not api_v3.plugin_store_manager: + return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500 - data = request.get_json(silent=True) - if not data or 'repo_url' not in data: - return jsonify({'status': 'error', 'message': 'repo_url required'}), 400 + data = request.get_json(silent=True) + if not data or 'repo_url' not in data: + return jsonify({'status': 'error', 'message': 'repo_url required'}), 400 - # A non-string repo_url is a client mistake, not a server fault: - # .strip() would raise and the catch-all would report it as a 500. - if not isinstance(data['repo_url'], str) or not data['repo_url'].strip(): - return jsonify({'status': 'error', 'message': 'repo_url must be a non-empty string'}), 400 + # A non-string repo_url is a client mistake, not a server fault: + # .strip() would raise and the catch-all would report it as a 500. + if not isinstance(data['repo_url'], str) or not data['repo_url'].strip(): + return jsonify({'status': 'error', 'message': 'repo_url must be a non-empty string'}), 400 - repo_url = data['repo_url'].strip() - plugin_id = data.get('plugin_id') # Optional, for monorepo installations - plugin_path = data.get('plugin_path') # Optional, for monorepo subdirectory - branch = data.get('branch') # Optional branch parameter + repo_url = data['repo_url'].strip() + plugin_id = data.get('plugin_id') # Optional, for monorepo installations + plugin_path = data.get('plugin_path') # Optional, for monorepo subdirectory + branch = data.get('branch') # Optional branch parameter - # Install the plugin - result = api_v3.plugin_store_manager.install_from_url( - repo_url=repo_url, - plugin_id=plugin_id, - plugin_path=plugin_path, - branch=branch - ) + # Install the plugin + result = api_v3.plugin_store_manager.install_from_url( + repo_url=repo_url, + plugin_id=plugin_id, + plugin_path=plugin_path, + branch=branch + ) - if result.get('success'): - # Invalidate schema cache for the installed plugin - installed_plugin_id = result.get('plugin_id') - if api_v3.schema_manager and installed_plugin_id: - api_v3.schema_manager.invalidate_cache(installed_plugin_id) + if result.get('success'): + # Invalidate schema cache for the installed plugin + installed_plugin_id = result.get('plugin_id') + if api_v3.schema_manager and installed_plugin_id: + api_v3.schema_manager.invalidate_cache(installed_plugin_id) - # Discover and load the new plugin - if api_v3.plugin_manager and installed_plugin_id: - api_v3.plugin_manager.discover_plugins() - api_v3.plugin_manager.load_plugin(installed_plugin_id) + # Discover and load the new plugin + if api_v3.plugin_manager and installed_plugin_id: + api_v3.plugin_manager.discover_plugins() + api_v3.plugin_manager.load_plugin(installed_plugin_id) - branch_msg = f" (branch: {result.get('branch', branch)})" if (result.get('branch') or branch) else "" - response_data = { - 'status': 'success', - 'message': f"Plugin {installed_plugin_id} installed successfully{branch_msg}", - 'plugin_id': installed_plugin_id, - 'name': result.get('name') - } - if result.get('branch'): - response_data['branch'] = result.get('branch') - return jsonify(response_data) - else: - return jsonify({ - 'status': 'error', - 'message': result.get('error', 'Failed to install plugin from URL') - }), 500 + branch_msg = f" (branch: {result.get('branch', branch)})" if (result.get('branch') or branch) else "" + response_data = { + 'status': 'success', + 'message': f"Plugin {installed_plugin_id} installed successfully{branch_msg}", + 'plugin_id': installed_plugin_id, + 'name': result.get('name') + } + if result.get('branch'): + response_data['branch'] = result.get('branch') + return jsonify(response_data) + else: + return jsonify({ + 'status': 'error', + 'message': result.get('error', 'Failed to install plugin from URL') + }), 500 - except Exception as e: - logger.error('Error in install_plugin_from_url', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 @api_v3.route('/plugins/registry-from-url', methods=['POST']) def get_registry_from_url(): """Get plugin list from a registry-style monorepo URL""" - try: - if not api_v3.plugin_store_manager: - return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500 + if not api_v3.plugin_store_manager: + return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500 - data = request.get_json(silent=True) - if not data or 'repo_url' not in data: - return jsonify({'status': 'error', 'message': 'repo_url required'}), 400 + data = request.get_json(silent=True) + if not data or 'repo_url' not in data: + return jsonify({'status': 'error', 'message': 'repo_url required'}), 400 - # A non-string repo_url is a client mistake, not a server fault: - # .strip() would raise and the catch-all would report it as a 500. - if not isinstance(data['repo_url'], str) or not data['repo_url'].strip(): - return jsonify({'status': 'error', 'message': 'repo_url must be a non-empty string'}), 400 + # A non-string repo_url is a client mistake, not a server fault: + # .strip() would raise and the catch-all would report it as a 500. + if not isinstance(data['repo_url'], str) or not data['repo_url'].strip(): + return jsonify({'status': 'error', 'message': 'repo_url must be a non-empty string'}), 400 - repo_url = data['repo_url'].strip() + repo_url = data['repo_url'].strip() - # Get registry from the URL - registry = api_v3.plugin_store_manager.fetch_registry_from_url(repo_url) + # Get registry from the URL + registry = api_v3.plugin_store_manager.fetch_registry_from_url(repo_url) - if registry: - return jsonify({ - 'status': 'success', - 'plugins': [p for p in registry.get('plugins', []) - if api_v3.plugin_store_manager.is_plugin_entry(p)], - 'registry_url': repo_url - }) - else: - return jsonify({ - 'status': 'error', - 'message': 'Failed to fetch registry from URL or URL does not contain a valid registry' - }), 400 + if registry: + return jsonify({ + 'status': 'success', + 'plugins': [p for p in registry.get('plugins', []) + if api_v3.plugin_store_manager.is_plugin_entry(p)], + 'registry_url': repo_url + }) + else: + return jsonify({ + 'status': 'error', + 'message': 'Failed to fetch registry from URL or URL does not contain a valid registry' + }), 400 - except Exception as e: - logger.error('Error in get_registry_from_url', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 @api_v3.route('/plugins/saved-repositories', methods=['GET']) def get_saved_repositories(): """Get all saved repositories""" - try: - if not api_v3.saved_repositories_manager: - return jsonify({'status': 'error', 'message': 'Saved repositories manager not initialized'}), 500 + if not api_v3.saved_repositories_manager: + return jsonify({'status': 'error', 'message': 'Saved repositories manager not initialized'}), 500 - repositories = api_v3.saved_repositories_manager.get_all() - return jsonify({'status': 'success', 'data': {'repositories': repositories}}) - except Exception as e: - logger.error('Error in get_saved_repositories', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + repositories = api_v3.saved_repositories_manager.get_all() + return jsonify({'status': 'success', 'data': {'repositories': repositories}}) @api_v3.route('/plugins/saved-repositories', methods=['POST']) def add_saved_repository(): """Add a repository to saved list""" - try: - if not api_v3.saved_repositories_manager: - return jsonify({'status': 'error', 'message': 'Saved repositories manager not initialized'}), 500 + if not api_v3.saved_repositories_manager: + return jsonify({'status': 'error', 'message': 'Saved repositories manager not initialized'}), 500 - data = request.get_json(silent=True) - if not data or 'repo_url' not in data: - return jsonify({'status': 'error', 'message': 'repo_url required'}), 400 + data = request.get_json(silent=True) + if not data or 'repo_url' not in data: + return jsonify({'status': 'error', 'message': 'repo_url required'}), 400 - # A non-string repo_url is a client mistake, not a server fault: - # .strip() would raise and the catch-all would report it as a 500. - if not isinstance(data['repo_url'], str) or not data['repo_url'].strip(): - return jsonify({'status': 'error', 'message': 'repo_url must be a non-empty string'}), 400 + # A non-string repo_url is a client mistake, not a server fault: + # .strip() would raise and the catch-all would report it as a 500. + if not isinstance(data['repo_url'], str) or not data['repo_url'].strip(): + return jsonify({'status': 'error', 'message': 'repo_url must be a non-empty string'}), 400 - repo_url = data['repo_url'].strip() - name = data.get('name') + repo_url = data['repo_url'].strip() + name = data.get('name') - success = api_v3.saved_repositories_manager.add(repo_url, name) + success = api_v3.saved_repositories_manager.add(repo_url, name) - if success: - return jsonify({ - 'status': 'success', - 'message': 'Repository saved successfully', - 'data': {'repositories': api_v3.saved_repositories_manager.get_all()} - }) - else: - return jsonify({ - 'status': 'error', - 'message': 'Repository already exists or failed to save' - }), 400 - except Exception as e: - logger.error('Error in add_saved_repository', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + if success: + return jsonify({ + 'status': 'success', + 'message': 'Repository saved successfully', + 'data': {'repositories': api_v3.saved_repositories_manager.get_all()} + }) + else: + return jsonify({ + 'status': 'error', + 'message': 'Repository already exists or failed to save' + }), 400 @api_v3.route('/plugins/saved-repositories', methods=['DELETE']) def remove_saved_repository(): """Remove a repository from saved list""" - try: - if not api_v3.saved_repositories_manager: - return jsonify({'status': 'error', 'message': 'Saved repositories manager not initialized'}), 500 + if not api_v3.saved_repositories_manager: + return jsonify({'status': 'error', 'message': 'Saved repositories manager not initialized'}), 500 - data = request.get_json(silent=True) - if not data or 'repo_url' not in data: - return jsonify({'status': 'error', 'message': 'repo_url required'}), 400 + data = request.get_json(silent=True) + if not data or 'repo_url' not in data: + return jsonify({'status': 'error', 'message': 'repo_url required'}), 400 - repo_url = data['repo_url'] + repo_url = data['repo_url'] - success = api_v3.saved_repositories_manager.remove(repo_url) + success = api_v3.saved_repositories_manager.remove(repo_url) - if success: - return jsonify({ - 'status': 'success', - 'message': 'Repository removed successfully', - 'data': {'repositories': api_v3.saved_repositories_manager.get_all()} - }) - else: - return jsonify({ - 'status': 'error', - 'message': 'Repository not found' - }), 404 - except Exception as e: - logger.error('Error in remove_saved_repository', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + if success: + return jsonify({ + 'status': 'success', + 'message': 'Repository removed successfully', + 'data': {'repositories': api_v3.saved_repositories_manager.get_all()} + }) + else: + return jsonify({ + 'status': 'error', + 'message': 'Repository not found' + }), 404 @api_v3.route('/plugins/store/list', methods=['GET']) def list_plugin_store(): """Search plugin store""" - try: - if not api_v3.plugin_store_manager: - return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500 + if not api_v3.plugin_store_manager: + return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500 - query = request.args.get('query', '') - category = request.args.get('category', '') - tags = request.args.getlist('tags') - # Default to fetching commit metadata to ensure accurate commit timestamps - fetch_commit_param = request.args.get('fetch_commit_info', request.args.get('fetch_latest_versions', '')).lower() - fetch_commit = fetch_commit_param != 'false' + query = request.args.get('query', '') + category = request.args.get('category', '') + tags = request.args.getlist('tags') + # Default to fetching commit metadata to ensure accurate commit timestamps + fetch_commit_param = request.args.get('fetch_commit_info', request.args.get('fetch_latest_versions', '')).lower() + fetch_commit = fetch_commit_param != 'false' - # Search plugins from the registry (including saved repositories) - plugins = api_v3.plugin_store_manager.search_plugins( - query=query, - category=category, - tags=tags, - fetch_commit_info=fetch_commit, - include_saved_repos=True, - saved_repositories_manager=api_v3.saved_repositories_manager - ) + # Search plugins from the registry (including saved repositories) + plugins = api_v3.plugin_store_manager.search_plugins( + query=query, + category=category, + tags=tags, + fetch_commit_info=fetch_commit, + include_saved_repos=True, + saved_repositories_manager=api_v3.saved_repositories_manager + ) - # Format plugins for the web interface - formatted_plugins = [] - for plugin in plugins: - if not api_v3.plugin_store_manager.is_plugin_entry(plugin): - continue - formatted_plugins.append({ - 'id': plugin.get('id'), - 'name': plugin.get('name'), - 'author': plugin.get('author'), - 'category': plugin.get('category'), - 'description': plugin.get('description'), - 'tags': plugin.get('tags', []), - 'stars': plugin.get('stars', 0), - 'verified': plugin.get('verified', False), - 'repo': plugin.get('repo', ''), - 'last_updated': plugin.get('last_updated') or plugin.get('last_updated_iso', ''), - 'last_updated_iso': plugin.get('last_updated_iso', ''), - 'last_commit': plugin.get('last_commit') or plugin.get('last_commit_sha'), - 'last_commit_message': plugin.get('last_commit_message'), - 'last_commit_author': plugin.get('last_commit_author'), - 'version': plugin.get('latest_version') or plugin.get('version', ''), - 'branch': plugin.get('branch') or plugin.get('default_branch'), - 'default_branch': plugin.get('default_branch'), - 'plugin_path': plugin.get('plugin_path', '') - }) + # Format plugins for the web interface + formatted_plugins = [] + for plugin in plugins: + if not api_v3.plugin_store_manager.is_plugin_entry(plugin): + continue + formatted_plugins.append({ + 'id': plugin.get('id'), + 'name': plugin.get('name'), + 'author': plugin.get('author'), + 'category': plugin.get('category'), + 'description': plugin.get('description'), + 'tags': plugin.get('tags', []), + 'stars': plugin.get('stars', 0), + 'verified': plugin.get('verified', False), + 'repo': plugin.get('repo', ''), + 'last_updated': plugin.get('last_updated') or plugin.get('last_updated_iso', ''), + 'last_updated_iso': plugin.get('last_updated_iso', ''), + 'last_commit': plugin.get('last_commit') or plugin.get('last_commit_sha'), + 'last_commit_message': plugin.get('last_commit_message'), + 'last_commit_author': plugin.get('last_commit_author'), + 'version': plugin.get('latest_version') or plugin.get('version', ''), + 'branch': plugin.get('branch') or plugin.get('default_branch'), + 'default_branch': plugin.get('default_branch'), + 'plugin_path': plugin.get('plugin_path', '') + }) - return jsonify({'status': 'success', 'data': {'plugins': formatted_plugins}}) - except Exception as e: - logger.error('Error in list_plugin_store', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + return jsonify({'status': 'success', 'data': {'plugins': formatted_plugins}}) @api_v3.route('/plugins/store/github-status', methods=['GET']) def get_github_auth_status(): """Check if GitHub authentication is configured and validate token""" - try: - if not api_v3.plugin_store_manager: - return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500 + if not api_v3.plugin_store_manager: + return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500 - token = api_v3.plugin_store_manager.github_token + token = api_v3.plugin_store_manager.github_token - # Check if GitHub token is configured - if not token or len(token) == 0: - return jsonify({ - 'status': 'success', - 'data': { - 'token_status': 'none', - 'authenticated': False, - 'rate_limit': 60, - 'message': 'No GitHub token configured', - 'error': None - } - }) + # Check if GitHub token is configured + if not token or len(token) == 0: + return jsonify({ + 'status': 'success', + 'data': { + 'token_status': 'none', + 'authenticated': False, + 'rate_limit': 60, + 'message': 'No GitHub token configured', + 'error': None + } + }) - # Validate the token - is_valid, error_message = api_v3.plugin_store_manager._validate_github_token(token) + # Validate the token + is_valid, error_message = api_v3.plugin_store_manager._validate_github_token(token) - if is_valid: - return jsonify({ - 'status': 'success', - 'data': { - 'token_status': 'valid', - 'authenticated': True, - 'rate_limit': 5000, - 'message': 'GitHub API authenticated', - 'error': None - } - }) - else: - return jsonify({ - 'status': 'success', - 'data': { - 'token_status': 'invalid', - 'authenticated': False, - 'rate_limit': 60, - 'message': f'GitHub token is invalid: {error_message}' if error_message else 'GitHub token is invalid', - 'error': error_message - } - }) - except Exception as e: - logger.error('Error in get_github_auth_status', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + if is_valid: + return jsonify({ + 'status': 'success', + 'data': { + 'token_status': 'valid', + 'authenticated': True, + 'rate_limit': 5000, + 'message': 'GitHub API authenticated', + 'error': None + } + }) + else: + return jsonify({ + 'status': 'success', + 'data': { + 'token_status': 'invalid', + 'authenticated': False, + 'rate_limit': 60, + 'message': f'GitHub token is invalid: {error_message}' if error_message else 'GitHub token is invalid', + 'error': error_message + } + }) @api_v3.route('/plugins/store/refresh', methods=['POST']) def refresh_plugin_store(): """Refresh plugin store repository""" - try: - if not api_v3.plugin_store_manager: - return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500 + if not api_v3.plugin_store_manager: + return jsonify({'status': 'error', 'message': 'Plugin store manager not initialized'}), 500 - data = request.get_json(silent=True) or {} - fetch_commit_info = data.get('fetch_commit_info', data.get('fetch_latest_versions', False)) + data = request.get_json(silent=True) or {} + fetch_commit_info = data.get('fetch_commit_info', data.get('fetch_latest_versions', False)) - # Force refresh the registry - registry = api_v3.plugin_store_manager.fetch_registry(force_refresh=True) - plugin_count = len(registry.get('plugins', [])) + # Force refresh the registry + registry = api_v3.plugin_store_manager.fetch_registry(force_refresh=True) + plugin_count = len(registry.get('plugins', [])) - message = 'Plugin store refreshed' - if fetch_commit_info: - message += ' (with refreshed commit metadata from GitHub)' + message = 'Plugin store refreshed' + if fetch_commit_info: + message += ' (with refreshed commit metadata from GitHub)' - return jsonify({ - 'status': 'success', - 'message': message, - 'plugin_count': plugin_count - }) - except Exception as e: - logger.error('Error in refresh_plugin_store', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + return jsonify({ + 'status': 'success', + 'message': message, + 'plugin_count': plugin_count + }) @api_v3.route('/plugins/config', methods=['POST']) def save_plugin_config(): """Save plugin configuration, separating secrets from regular config""" @@ -2308,8 +2190,6 @@ def save_plugin_config(): return success_response(message=message) except Exception as e: - from src.web_interface.errors import WebInterfaceError - error = WebInterfaceError.from_exception(e, ErrorCode.CONFIG_SAVE_FAILED) if api_v3.operation_history: api_v3.operation_history.record_operation( "configure", @@ -2317,13 +2197,7 @@ def save_plugin_config(): status="failed", error=str(e) ) - return error_response( - error.error_code, - error.message, - details=error.details, - context=error.context, - status_code=500 - ) + return exception_error_response(e, ErrorCode.CONFIG_SAVE_FAILED) def _merge_onto_stored_plugin_config(plugin_id, submitted_config, current_config=None): """A JSON plugin-config body merged onto the plugin's stored section. @@ -2740,126 +2614,118 @@ def _prepare_plugin_config_for_save(plugin_id, plugin_config, schema, schema_mgr @api_v3.route('/plugins/schema', methods=['GET']) def get_plugin_schema(): """Get plugin configuration schema""" - try: - plugin_id = request.args.get('plugin_id') - if not plugin_id: - return jsonify({'status': 'error', 'message': 'plugin_id required'}), 400 + plugin_id = request.args.get('plugin_id') + if not plugin_id: + return jsonify({'status': 'error', 'message': 'plugin_id required'}), 400 - # Get schema manager instance - schema_mgr = api_v3.schema_manager - if not schema_mgr: - return jsonify({'status': 'error', 'message': 'Schema manager not initialized'}), 500 + # Get schema manager instance + schema_mgr = api_v3.schema_manager + if not schema_mgr: + return jsonify({'status': 'error', 'message': 'Schema manager not initialized'}), 500 - # Load schema using SchemaManager (uses caching) - schema = schema_mgr.load_schema(plugin_id, use_cache=True) + # Load schema using SchemaManager (uses caching) + schema = schema_mgr.load_schema(plugin_id, use_cache=True) - if schema: - return jsonify({'status': 'success', 'data': {'schema': schema}}) + if schema: + return jsonify({'status': 'success', 'data': {'schema': schema}}) - # Return a simple default schema if file not found - default_schema = { - 'type': 'object', - 'properties': { - 'enabled': { - 'type': 'boolean', - 'title': 'Enable Plugin', - 'description': 'Enable or disable this plugin', - 'default': True - }, - 'display_duration': { - 'type': 'integer', - 'title': 'Display Duration', - 'description': 'How long to show content (seconds)', - 'minimum': 5, - 'maximum': 300, - 'default': 30 - } + # Return a simple default schema if file not found + default_schema = { + 'type': 'object', + 'properties': { + 'enabled': { + 'type': 'boolean', + 'title': 'Enable Plugin', + 'description': 'Enable or disable this plugin', + 'default': True + }, + 'display_duration': { + 'type': 'integer', + 'title': 'Display Duration', + 'description': 'How long to show content (seconds)', + 'minimum': 5, + 'maximum': 300, + 'default': 30 } } + } - return jsonify({'status': 'success', 'data': {'schema': default_schema}}) - except Exception as e: - logger.error('Error in get_plugin_schema', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + return jsonify({'status': 'success', 'data': {'schema': default_schema}}) @api_v3.route('/plugins/config/reset', methods=['POST']) def reset_plugin_config(): """Reset plugin configuration to schema defaults""" - try: - if not api_v3.config_manager: - return jsonify({'status': 'error', 'message': 'Config manager not initialized'}), 500 + if not api_v3.config_manager: + return jsonify({'status': 'error', 'message': 'Config manager not initialized'}), 500 - data = request.get_json(silent=True) or {} - plugin_id = data.get('plugin_id') - preserve_secrets = data.get('preserve_secrets', True) + data = request.get_json(silent=True) or {} + plugin_id = data.get('plugin_id') + preserve_secrets = data.get('preserve_secrets', True) - if not plugin_id: - return jsonify({'status': 'error', 'message': 'plugin_id required'}), 400 + if not plugin_id: + return jsonify({'status': 'error', 'message': 'plugin_id required'}), 400 - # Get schema manager instance - schema_mgr = api_v3.schema_manager - if not schema_mgr: - return jsonify({'status': 'error', 'message': 'Schema manager not initialized'}), 500 + # Get schema manager instance + schema_mgr = api_v3.schema_manager + if not schema_mgr: + return jsonify({'status': 'error', 'message': 'Schema manager not initialized'}), 500 - # Generate defaults from schema - defaults = schema_mgr.generate_default_config(plugin_id, use_cache=True) + # Generate defaults from schema + defaults = schema_mgr.generate_default_config(plugin_id, use_cache=True) - # Get current configs - current_config = api_v3.config_manager.load_config() - current_secrets = api_v3.config_manager.get_raw_file_content('secrets') + # Get current configs + current_config = api_v3.config_manager.load_config() + current_secrets = api_v3.config_manager.get_raw_file_content('secrets') - # Load schema to identify secret fields - schema = schema_mgr.load_schema(plugin_id, use_cache=True) - secret_fields = set() + # Load schema to identify secret fields + schema = schema_mgr.load_schema(plugin_id, use_cache=True) + secret_fields = set() - if schema and 'properties' in schema: - secret_fields = find_secret_fields(schema['properties']) + if schema and 'properties' in schema: + secret_fields = find_secret_fields(schema['properties']) - # Separate defaults into regular and secret configs - default_regular, default_secrets = separate_secrets(defaults, secret_fields) + # Separate defaults into regular and secret configs + default_regular, default_secrets = separate_secrets(defaults, secret_fields) - # Update main config with defaults - current_config[plugin_id] = default_regular + # Update main config with defaults + current_config[plugin_id] = default_regular - # Update secrets config (preserve existing secrets if preserve_secrets=True) - if preserve_secrets: - # Keep existing secrets for this plugin - if plugin_id in current_secrets: - # Merge defaults with existing secrets - existing_secrets = current_secrets[plugin_id] - for key, value in default_secrets.items(): - if key not in existing_secrets or not existing_secrets[key]: - existing_secrets[key] = value - else: - current_secrets[plugin_id] = default_secrets + # Update secrets config (preserve existing secrets if preserve_secrets=True) + if preserve_secrets: + # Keep existing secrets for this plugin + if plugin_id in current_secrets: + # Merge defaults with existing secrets + existing_secrets = current_secrets[plugin_id] + for key, value in default_secrets.items(): + if key not in existing_secrets or not existing_secrets[key]: + existing_secrets[key] = value else: - # Replace all secrets with defaults current_secrets[plugin_id] = default_secrets + else: + # Replace all secrets with defaults + current_secrets[plugin_id] = default_secrets - # Save updated configs - api_v3.config_manager.save_config(current_config) - if default_secrets or not preserve_secrets: - api_v3.config_manager.save_raw_file_content('secrets', current_secrets) + # Save updated configs + api_v3.config_manager.save_config(current_config) + if default_secrets or not preserve_secrets: + api_v3.config_manager.save_raw_file_content('secrets', current_secrets) - # Notify plugin of config change if loaded - try: - if api_v3.plugin_manager: - plugin_instance = api_v3.plugin_manager.get_plugin(plugin_id) - if plugin_instance: - merged_config = api_v3.config_manager.load_config() - plugin_full_config = merged_config.get(plugin_id, {}) - if hasattr(plugin_instance, 'on_config_change'): - plugin_instance.on_config_change(plugin_full_config) - except Exception as hook_err: - logger.warning("on_config_change failed: %s", hook_err) + # Notify plugin of config change if loaded + try: + if api_v3.plugin_manager: + plugin_instance = api_v3.plugin_manager.get_plugin(plugin_id) + if plugin_instance: + merged_config = api_v3.config_manager.load_config() + plugin_full_config = merged_config.get(plugin_id, {}) + if hasattr(plugin_instance, 'on_config_change'): + plugin_instance.on_config_change(plugin_full_config) + except Exception as hook_err: + logger.warning("on_config_change failed: %s", hook_err) - return jsonify({ - 'status': 'success', - 'message': f'Plugin {plugin_id} configuration reset to defaults', - 'data': {'config': defaults} - }) - except Exception as e: - logger.error('Error in reset_plugin_config', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + return jsonify({ + 'status': 'success', + 'message': f'Plugin {plugin_id} configuration reset to defaults', + 'data': {'config': defaults} + }) @api_v3.route('/plugins/action', methods=['POST']) def execute_plugin_action(): """Execute a plugin-defined action (e.g., authentication)""" @@ -2868,8 +2734,10 @@ def execute_plugin_action(): try: data = request.get_json(force=True) or {} except Exception as e: - import logging - logger = logging.getLogger(__name__) + # The module logger, not a local one: binding `logger` anywhere in + # this function made every other `logger.error` here raise + # UnboundLocalError, so the step-1 handler below reported that + # instead of the plugin script's real failure. logger.error(f"Error parsing JSON in execute_plugin_action: {e}") return jsonify({ 'status': 'error', @@ -3172,9 +3040,6 @@ sys.exit(proc.returncode) except subprocess.TimeoutExpired: return jsonify({'status': 'error', 'message': 'Action timed out'}), 408 - except Exception as e: - logger.error('Error in execute_plugin_action', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 def _plugin_uploads_dir(plugin_id): """assets/plugins//uploads for a request-supplied id, or None. @@ -3187,154 +3052,150 @@ def _plugin_uploads_dir(plugin_id): @api_v3.route('/plugins/assets/upload', methods=['POST']) def upload_plugin_asset(): """Upload asset files for a plugin""" - try: - plugin_id = request.form.get('plugin_id') - if not plugin_id: - return jsonify({'status': 'error', 'message': 'plugin_id is required'}), 400 + plugin_id = request.form.get('plugin_id') + if not plugin_id: + return jsonify({'status': 'error', 'message': 'plugin_id is required'}), 400 - if 'files' not in request.files: - return jsonify({'status': 'error', 'message': 'No files provided'}), 400 + if 'files' not in request.files: + return jsonify({'status': 'error', 'message': 'No files provided'}), 400 - files = request.files.getlist('files') - if not files or all(not f.filename for f in files): - return jsonify({'status': 'error', 'message': 'No files provided'}), 400 + files = request.files.getlist('files') + if not files or all(not f.filename for f in files): + return jsonify({'status': 'error', 'message': 'No files provided'}), 400 - # Validate file count - if len(files) > 10: - return jsonify({'status': 'error', 'message': 'Maximum 10 files per upload'}), 400 + # Validate file count + if len(files) > 10: + return jsonify({'status': 'error', 'message': 'Maximum 10 files per upload'}), 400 - # Setup plugin assets directory. plugin_id is a form field: without - # the guard '../../config' created, listed and wrote into directories - # outside assets/plugins (the serving route was fixed in #561). - assets_dir = _plugin_uploads_dir(plugin_id) - if assets_dir is None: - return jsonify({'status': 'error', 'message': 'Invalid plugin_id'}), 400 - plugin_id = safe_path_component(plugin_id) - assets_dir.mkdir(parents=True, exist_ok=True) + # Setup plugin assets directory. plugin_id is a form field: without + # the guard '../../config' created, listed and wrote into directories + # outside assets/plugins (the serving route was fixed in #561). + assets_dir = _plugin_uploads_dir(plugin_id) + if assets_dir is None: + return jsonify({'status': 'error', 'message': 'Invalid plugin_id'}), 400 + plugin_id = safe_path_component(plugin_id) + assets_dir.mkdir(parents=True, exist_ok=True) - # Load metadata file - metadata_file = assets_dir / '.metadata.json' - if metadata_file.exists(): - with open(metadata_file, 'r') as f: - metadata = json.load(f) - else: - metadata = {} + # Load metadata file + metadata_file = assets_dir / '.metadata.json' + if metadata_file.exists(): + with open(metadata_file, 'r') as f: + metadata = json.load(f) + else: + metadata = {} - uploaded_files = [] - total_size = 0 - max_size_per_file = 5 * 1024 * 1024 # 5MB - max_total_size = 50 * 1024 * 1024 # 50MB + uploaded_files = [] + total_size = 0 + max_size_per_file = 5 * 1024 * 1024 # 5MB + max_total_size = 50 * 1024 * 1024 # 50MB - # Calculate current total size - for entry in metadata.values(): - if 'size' in entry: - total_size += entry.get('size', 0) + # Calculate current total size + for entry in metadata.values(): + if 'size' in entry: + total_size += entry.get('size', 0) - for file in files: - if not file.filename: - continue + for file in files: + if not file.filename: + continue - # Validate file type - allowed_extensions = ['.png', '.jpg', '.jpeg', '.bmp', '.gif'] - file_ext = '.' + file.filename.lower().split('.')[-1] - if file_ext not in allowed_extensions: - return jsonify({ - 'status': 'error', - 'message': f'Invalid file type: {file_ext}. Allowed: {allowed_extensions}' - }), 400 + # Validate file type + allowed_extensions = ['.png', '.jpg', '.jpeg', '.bmp', '.gif'] + file_ext = '.' + file.filename.lower().split('.')[-1] + if file_ext not in allowed_extensions: + return jsonify({ + 'status': 'error', + 'message': f'Invalid file type: {file_ext}. Allowed: {allowed_extensions}' + }), 400 - # Read file to check size and validate - file.seek(0, os.SEEK_END) - file_size = file.tell() - file.seek(0) + # Read file to check size and validate + file.seek(0, os.SEEK_END) + file_size = file.tell() + file.seek(0) - if file_size > max_size_per_file: - return jsonify({ - 'status': 'error', - 'message': f'File {file.filename} exceeds 5MB limit' - }), 400 + if file_size > max_size_per_file: + return jsonify({ + 'status': 'error', + 'message': f'File {file.filename} exceeds 5MB limit' + }), 400 - if total_size + file_size > max_total_size: - return jsonify({ - 'status': 'error', - 'message': f'Upload would exceed 50MB total storage limit' - }), 400 + if total_size + file_size > max_total_size: + return jsonify({ + 'status': 'error', + 'message': f'Upload would exceed 50MB total storage limit' + }), 400 - # Validate file is actually an image (check magic bytes) - file_content = file.read(8) - file.seek(0) - is_valid_image = False - if file_content.startswith(b'\x89PNG\r\n\x1a\n'): # PNG - is_valid_image = True - elif file_content[:2] == b'\xff\xd8': # JPEG - is_valid_image = True - elif file_content[:2] == b'BM': # BMP - is_valid_image = True - elif file_content[:6] in [b'GIF87a', b'GIF89a']: # GIF - is_valid_image = True + # Validate file is actually an image (check magic bytes) + file_content = file.read(8) + file.seek(0) + is_valid_image = False + if file_content.startswith(b'\x89PNG\r\n\x1a\n'): # PNG + is_valid_image = True + elif file_content[:2] == b'\xff\xd8': # JPEG + is_valid_image = True + elif file_content[:2] == b'BM': # BMP + is_valid_image = True + elif file_content[:6] in [b'GIF87a', b'GIF89a']: # GIF + is_valid_image = True - if not is_valid_image: - return jsonify({ - 'status': 'error', - 'message': f'File {file.filename} is not a valid image file' - }), 400 + if not is_valid_image: + return jsonify({ + 'status': 'error', + 'message': f'File {file.filename} is not a valid image file' + }), 400 - # Generate unique filename - timestamp = int(_pkg.time.time()) - file_hash = hashlib.md5(file_content + file.filename.encode()).hexdigest()[:8] - safe_filename = f"image_{timestamp}_{file_hash}{file_ext}" + # Generate unique filename + timestamp = int(_pkg.time.time()) + file_hash = hashlib.md5(file_content + file.filename.encode()).hexdigest()[:8] + safe_filename = f"image_{timestamp}_{file_hash}{file_ext}" + file_path = assets_dir / safe_filename + + # Ensure filename is unique + counter = 1 + while file_path.exists(): + safe_filename = f"image_{timestamp}_{file_hash}_{counter}{file_ext}" file_path = assets_dir / safe_filename + counter += 1 - # Ensure filename is unique - counter = 1 - while file_path.exists(): - safe_filename = f"image_{timestamp}_{file_hash}_{counter}{file_ext}" - file_path = assets_dir / safe_filename - counter += 1 + # Save file + file.save(str(file_path)) - # Save file - file.save(str(file_path)) + # Make file readable + os.chmod(file_path, 0o644) - # Make file readable - os.chmod(file_path, 0o644) + # Generate unique ID + image_id = str(uuid.uuid4()) - # Generate unique ID - image_id = str(uuid.uuid4()) + # Store metadata + relative_path = f"assets/plugins/{plugin_id}/uploads/{safe_filename}" + metadata[image_id] = { + 'id': image_id, + 'filename': safe_filename, + 'path': relative_path, + 'size': file_size, + 'uploaded_at': datetime.utcnow().isoformat() + 'Z', + 'original_filename': file.filename + } - # Store metadata - relative_path = f"assets/plugins/{plugin_id}/uploads/{safe_filename}" - metadata[image_id] = { - 'id': image_id, - 'filename': safe_filename, - 'path': relative_path, - 'size': file_size, - 'uploaded_at': datetime.utcnow().isoformat() + 'Z', - 'original_filename': file.filename - } - - uploaded_files.append({ - 'id': image_id, - 'filename': safe_filename, - 'path': relative_path, - 'size': file_size, - 'uploaded_at': metadata[image_id]['uploaded_at'] - }) - - total_size += file_size - - # Save metadata - with open(metadata_file, 'w') as f: - json.dump(metadata, f, indent=2) - - return jsonify({ - 'status': 'success', - 'uploaded_files': uploaded_files, - 'total_files': len(metadata) + uploaded_files.append({ + 'id': image_id, + 'filename': safe_filename, + 'path': relative_path, + 'size': file_size, + 'uploaded_at': metadata[image_id]['uploaded_at'] }) - except Exception as e: - logger.error('Unhandled exception', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + total_size += file_size + + # Save metadata + with open(metadata_file, 'w') as f: + json.dump(metadata, f, indent=2) + + return jsonify({ + 'status': 'success', + 'uploaded_files': uploaded_files, + 'total_files': len(metadata) + }) + @api_v3.route('/plugins//static/', methods=['GET']) def serve_plugin_static(plugin_id, file_path): """Serve static files from plugin directory. @@ -3353,132 +3214,124 @@ def serve_plugin_static(plugin_id, file_path): ``plugin-repos/foo-evil/x``, whose string does start with ``plugin-repos/foo``. """ - try: - safe_plugin_id = safe_path_component(plugin_id) - if not safe_plugin_id: - return jsonify({'status': 'error', 'message': 'Invalid plugin ID'}), 400 + safe_plugin_id = safe_path_component(plugin_id) + if not safe_plugin_id: + return jsonify({'status': 'error', 'message': 'Invalid plugin ID'}), 400 - safe_parts = safe_relative_parts(file_path) - if not safe_parts: - return jsonify({'status': 'error', 'message': 'Invalid file path'}), 400 + safe_parts = safe_relative_parts(file_path) + if not safe_parts: + return jsonify({'status': 'error', 'message': 'Invalid file path'}), 400 - # Get plugin directory - if api_v3.plugin_manager: - plugin_dir = api_v3.plugin_manager.get_plugin_directory(safe_plugin_id) - else: - plugin_dir = PROJECT_ROOT / 'plugins' / safe_plugin_id + # Get plugin directory + if api_v3.plugin_manager: + plugin_dir = api_v3.plugin_manager.get_plugin_directory(safe_plugin_id) + else: + plugin_dir = PROJECT_ROOT / 'plugins' / safe_plugin_id - if not plugin_dir or not Path(plugin_dir).exists(): - return jsonify({'status': 'error', 'message': 'Plugin not found'}), 404 + if not plugin_dir or not Path(plugin_dir).exists(): + return jsonify({'status': 'error', 'message': 'Plugin not found'}), 404 - # Containment is still checked after resolving: name validation cannot - # see a symlink inside the plugin directory that points out of it. - requested_file = resolve_under(plugin_dir, *safe_parts) - if requested_file is None: - return jsonify({'status': 'error', 'message': 'Invalid file path'}), 403 + # Containment is still checked after resolving: name validation cannot + # see a symlink inside the plugin directory that points out of it. + requested_file = resolve_under(plugin_dir, *safe_parts) + if requested_file is None: + return jsonify({'status': 'error', 'message': 'Invalid file path'}), 403 - # Check if file exists - if not requested_file.exists() or not requested_file.is_file(): - return jsonify({'status': 'error', 'message': 'File not found'}), 404 + # Check if file exists + if not requested_file.exists() or not requested_file.is_file(): + return jsonify({'status': 'error', 'message': 'File not found'}), 404 - # Determine content type - content_type = 'text/plain' - name = requested_file.name - if name.endswith('.html'): - content_type = 'text/html' - elif name.endswith('.js'): - content_type = 'application/javascript' - elif name.endswith('.css'): - content_type = 'text/css' - elif name.endswith('.json'): - content_type = 'application/json' + # Determine content type + content_type = 'text/plain' + name = requested_file.name + if name.endswith('.html'): + content_type = 'text/html' + elif name.endswith('.js'): + content_type = 'application/javascript' + elif name.endswith('.css'): + content_type = 'text/css' + elif name.endswith('.json'): + content_type = 'application/json' - # Read and return file - with open(requested_file, 'r', encoding='utf-8') as f: - content = f.read() + # Read and return file + with open(requested_file, 'r', encoding='utf-8') as f: + content = f.read() - return Response(content, mimetype=content_type) + return Response(content, mimetype=content_type) - except Exception as e: - logger.error('Unhandled exception', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 @api_v3.route('/plugins/calendar/upload-credentials', methods=['POST']) def upload_calendar_credentials(): """Upload credentials.json file for calendar plugin""" + if 'file' not in request.files: + return jsonify({'status': 'error', 'message': 'No file provided'}), 400 + + file = request.files['file'] + if not file or not file.filename: + return jsonify({'status': 'error', 'message': 'No file provided'}), 400 + + # Validate file extension + if not file.filename.lower().endswith('.json'): + return jsonify({'status': 'error', 'message': 'File must be a JSON file (.json)'}), 400 + + # Validate file size (max 1MB for credentials) + file.seek(0, os.SEEK_END) + file_size = file.tell() + file.seek(0) + + if file_size > 1024 * 1024: # 1MB + return jsonify({'status': 'error', 'message': 'File exceeds 1MB limit'}), 400 + + # Validate it's valid JSON try: - if 'file' not in request.files: - return jsonify({'status': 'error', 'message': 'No file provided'}), 400 - - file = request.files['file'] - if not file or not file.filename: - return jsonify({'status': 'error', 'message': 'No file provided'}), 400 - - # Validate file extension - if not file.filename.lower().endswith('.json'): - return jsonify({'status': 'error', 'message': 'File must be a JSON file (.json)'}), 400 - - # Validate file size (max 1MB for credentials) - file.seek(0, os.SEEK_END) - file_size = file.tell() + file_content = file.read() file.seek(0) + creds_data = json.loads(file_content) + except json.JSONDecodeError: + return jsonify({'status': 'error', 'message': 'File is not valid JSON'}), 400 - if file_size > 1024 * 1024: # 1MB - return jsonify({'status': 'error', 'message': 'File exceeds 1MB limit'}), 400 - - # Validate it's valid JSON - try: - file_content = file.read() - file.seek(0) - creds_data = json.loads(file_content) - except json.JSONDecodeError: - return jsonify({'status': 'error', 'message': 'File is not valid JSON'}), 400 - - # Validate it looks like Google OAuth credentials. A bare scalar, a - # list, true/null — all valid JSON, none of them credentials. Reject - # rather than save: a file written as credentials.json but unusable - # as credentials only fails later, somewhere less obvious. - if not isinstance(creds_data, dict) or not ( - 'installed' in creds_data or 'web' in creds_data): - return jsonify({ - 'status': 'error', - 'message': 'File does not appear to be a valid Google OAuth credentials file' - }), 400 - - # Get plugin directory - plugin_id = 'calendar' - if api_v3.plugin_manager: - plugin_dir = api_v3.plugin_manager.get_plugin_directory(plugin_id) - else: - plugin_dir = PROJECT_ROOT / 'plugins' / plugin_id - - if not plugin_dir or not Path(plugin_dir).exists(): - return jsonify({'status': 'error', 'message': 'Plugin not found'}), 404 - - # Save file to plugin directory - credentials_path = Path(plugin_dir) / 'credentials.json' - - # Backup existing file if it exists - if credentials_path.exists(): - backup_path = Path(plugin_dir) / f'credentials.json.backup.{int(_pkg.time.time())}' - import shutil - shutil.copy2(credentials_path, backup_path) - _prune_credential_backups(Path(plugin_dir)) - - # Save new file - file.save(str(credentials_path)) - - # Set proper permissions - os.chmod(credentials_path, 0o600) # Read/write for owner only - + # Validate it looks like Google OAuth credentials. A bare scalar, a + # list, true/null — all valid JSON, none of them credentials. Reject + # rather than save: a file written as credentials.json but unusable + # as credentials only fails later, somewhere less obvious. + if not isinstance(creds_data, dict) or not ( + 'installed' in creds_data or 'web' in creds_data): return jsonify({ - 'status': 'success', - 'message': 'Credentials file uploaded successfully', - 'path': str(credentials_path) - }) + 'status': 'error', + 'message': 'File does not appear to be a valid Google OAuth credentials file' + }), 400 + + # Get plugin directory + plugin_id = 'calendar' + if api_v3.plugin_manager: + plugin_dir = api_v3.plugin_manager.get_plugin_directory(plugin_id) + else: + plugin_dir = PROJECT_ROOT / 'plugins' / plugin_id + + if not plugin_dir or not Path(plugin_dir).exists(): + return jsonify({'status': 'error', 'message': 'Plugin not found'}), 404 + + # Save file to plugin directory + credentials_path = Path(plugin_dir) / 'credentials.json' + + # Backup existing file if it exists + if credentials_path.exists(): + backup_path = Path(plugin_dir) / f'credentials.json.backup.{int(_pkg.time.time())}' + import shutil + shutil.copy2(credentials_path, backup_path) + _prune_credential_backups(Path(plugin_dir)) + + # Save new file + file.save(str(credentials_path)) + + # Set proper permissions + os.chmod(credentials_path, 0o600) # Read/write for owner only + + return jsonify({ + 'status': 'success', + 'message': 'Credentials file uploaded successfully', + 'path': str(credentials_path) + }) - except Exception as e: - logger.error('Error in upload_calendar_credentials', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 @api_v3.route('/plugins/calendar/authenticate', methods=['POST']) def authenticate_calendar(): """Google OAuth for the calendar plugin, in the two steps it requires. @@ -3492,44 +3345,38 @@ def authenticate_calendar(): The script persists the PKCE verifier from step 1 for step 2 to reuse; the exchange fails with "Missing code verifier" otherwise. """ - try: - plugin_dir = _pkg._calendar_plugin_dir() - if plugin_dir is None: - return jsonify({ - 'status': 'error', - 'message': 'The calendar plugin is not installed' - }), 404 + plugin_dir = _pkg._calendar_plugin_dir() + if plugin_dir is None: + return jsonify({ + 'status': 'error', + 'message': 'The calendar plugin is not installed' + }), 404 - if not (plugin_dir / 'credentials.json').exists(): - return jsonify({ - 'status': 'error', - 'message': ('No credentials.json yet. Upload your Google OAuth ' - 'client file first (Step 1).') - }), 400 + if not (plugin_dir / 'credentials.json').exists(): + return jsonify({ + 'status': 'error', + 'message': ('No credentials.json yet. Upload your Google OAuth ' + 'client file first (Step 1).') + }), 400 - data = request.get_json(silent=True) or {} - redirect_url = (data.get('redirect_url') or data.get('code') or '').strip() + data = request.get_json(silent=True) or {} + redirect_url = (data.get('redirect_url') or data.get('code') or '').strip() - payload, error = _run_calendar_registration(plugin_dir, redirect_url) - if error: - return jsonify({'status': 'error', 'message': error}), 500 - if payload.get('status') != 'success': - # The script's own diagnosis is more useful than anything that - # could be reconstructed here -- but it interpolates exceptions - # into its messages, so it reaches the client redacted and the - # original goes to the log. - logger.error('calendar authentication failed: %s', payload) - safe = dict(payload) - safe['message'] = redact_text(str(payload.get('message', '') - or 'Authentication failed')) - return jsonify(safe), 400 - return jsonify(payload) + payload, error = _run_calendar_registration(plugin_dir, redirect_url) + if error: + return jsonify({'status': 'error', 'message': error}), 500 + if payload.get('status') != 'success': + # The script's own diagnosis is more useful than anything that + # could be reconstructed here -- but it interpolates exceptions + # into its messages, so it reaches the client redacted and the + # original goes to the log. + logger.error('calendar authentication failed: %s', payload) + safe = dict(payload) + safe['message'] = redact_text(str(payload.get('message', '') + or 'Authentication failed')) + return jsonify(safe), 400 + return jsonify(payload) - except Exception as e: - logger.error('Error in authenticate_calendar', exc_info=True) - return jsonify({'status': 'error', - 'message': 'An error occurred; see logs for details', - 'details': describe_exception(e)}), 500 @api_v3.route('/plugins/calendar/list-calendars', methods=['GET']) def list_calendar_calendars(): """The calendars this account can see, for the config picker. @@ -3538,174 +3385,160 @@ def list_calendar_calendars(): picker is used interactively and a subprocess per click is slower than the API call it would be wrapping. """ + plugin_dir = _pkg._calendar_plugin_dir() + if plugin_dir is None: + return jsonify({ + 'status': 'error', + 'message': 'The calendar plugin is not installed' + }), 404 + + token_file = plugin_dir / 'token.pickle' + if not token_file.exists(): + return jsonify({ + 'status': 'error', + 'message': ('Not authenticated with Google yet. Complete Step 2 ' + 'first, then load your calendars.') + }), 400 + try: - plugin_dir = _pkg._calendar_plugin_dir() - if plugin_dir is None: - return jsonify({ - 'status': 'error', - 'message': 'The calendar plugin is not installed' - }), 404 + import pickle + from google.auth.transport.requests import Request as GoogleRequest + from googleapiclient.discovery import build as build_google_service + except ImportError as e: + return jsonify({ + 'status': 'error', + # The name of the missing module is the whole diagnosis, but it + # arrives as an exception, so it goes through the redactor like + # any other -- an ImportError can quote a path. + 'message': ('The Google API libraries are not installed. Install ' + "the calendar plugin's requirements.txt. (%s)" + % describe_exception(e)) + }), 500 - token_file = plugin_dir / 'token.pickle' - if not token_file.exists(): - return jsonify({ - 'status': 'error', - 'message': ('Not authenticated with Google yet. Complete Step 2 ' - 'first, then load your calendars.') - }), 400 + with open(token_file, 'rb') as handle: + # Written only by this plugin's own OAuth flow, into its own + # directory, and read here exactly as the plugin itself reads it. + creds = pickle.load(handle) # nosec B301 - locally generated token - try: - import pickle - from google.auth.transport.requests import Request as GoogleRequest - from googleapiclient.discovery import build as build_google_service - except ImportError as e: - return jsonify({ - 'status': 'error', - # The name of the missing module is the whole diagnosis, but it - # arrives as an exception, so it goes through the redactor like - # any other -- an ImportError can quote a path. - 'message': ('The Google API libraries are not installed. Install ' - "the calendar plugin's requirements.txt. (%s)" - % describe_exception(e)) - }), 500 + if creds and creds.expired and creds.refresh_token: + creds.refresh(GoogleRequest()) + with open(token_file, 'wb') as handle: + pickle.dump(creds, handle) + os.chmod(token_file, 0o600) - with open(token_file, 'rb') as handle: - # Written only by this plugin's own OAuth flow, into its own - # directory, and read here exactly as the plugin itself reads it. - creds = pickle.load(handle) # nosec B301 - locally generated token + if not creds or not creds.valid: + return jsonify({ + 'status': 'error', + 'message': ('Stored Google credentials are no longer valid. ' + 'Run Step 2 again to re-authenticate.') + }), 400 - if creds and creds.expired and creds.refresh_token: - creds.refresh(GoogleRequest()) - with open(token_file, 'wb') as handle: - pickle.dump(creds, handle) - os.chmod(token_file, 0o600) + service = build_google_service('calendar', 'v3', credentials=creds) - if not creds or not creds.valid: - return jsonify({ - 'status': 'error', - 'message': ('Stored Google credentials are no longer valid. ' - 'Run Step 2 again to re-authenticate.') - }), 400 + # calendarList.list returns 100 entries per page by default and caps at + # 250, handing back a nextPageToken when there are more. Taking only + # the first page would silently hide calendars from the picker, and the + # user would have no way to tell the list was truncated. + entries = [] + page_token = None + for _ in range(_CALENDAR_LIST_MAX_PAGES): + response = service.calendarList().list( + maxResults=250, pageToken=page_token).execute() + entries.extend(response.get('items', [])) + page_token = response.get('nextPageToken') + if not page_token: + break + else: + # 2500 calendars in, something is wrong with the account or the + # token is looping; show what was collected rather than spin. + logger.warning( + 'calendarList paging stopped at %d pages with more remaining', + _CALENDAR_LIST_MAX_PAGES) - service = build_google_service('calendar', 'v3', credentials=creds) + calendars = [{ + 'id': entry.get('id'), + # The picker labels each row with summary and falls back to the id + # only in its own display, so send something either way. + 'summary': entry.get('summary') or entry.get('id'), + 'primary': bool(entry.get('primary', False)), + } for entry in entries if entry.get('id')] - # calendarList.list returns 100 entries per page by default and caps at - # 250, handing back a nextPageToken when there are more. Taking only - # the first page would silently hide calendars from the picker, and the - # user would have no way to tell the list was truncated. - entries = [] - page_token = None - for _ in range(_CALENDAR_LIST_MAX_PAGES): - response = service.calendarList().list( - maxResults=250, pageToken=page_token).execute() - entries.extend(response.get('items', [])) - page_token = response.get('nextPageToken') - if not page_token: - break - else: - # 2500 calendars in, something is wrong with the account or the - # token is looping; show what was collected rather than spin. - logger.warning( - 'calendarList paging stopped at %d pages with more remaining', - _CALENDAR_LIST_MAX_PAGES) + # Primary first, then alphabetically: the list is usually short but the + # one the user wants is almost always their own calendar. + calendars.sort(key=lambda c: (not c['primary'], c['summary'].lower())) - calendars = [{ - 'id': entry.get('id'), - # The picker labels each row with summary and falls back to the id - # only in its own display, so send something either way. - 'summary': entry.get('summary') or entry.get('id'), - 'primary': bool(entry.get('primary', False)), - } for entry in entries if entry.get('id')] + return jsonify({'status': 'success', 'calendars': calendars}) - # Primary first, then alphabetically: the list is usually short but the - # one the user wants is almost always their own calendar. - calendars.sort(key=lambda c: (not c['primary'], c['summary'].lower())) - - return jsonify({'status': 'success', 'calendars': calendars}) - - except Exception as e: - logger.error('Error in list_calendar_calendars', exc_info=True) - return jsonify({'status': 'error', - 'message': 'An error occurred; see logs for details', - 'details': describe_exception(e)}), 500 @api_v3.route('/plugins/assets/delete', methods=['POST']) def delete_plugin_asset(): """Delete an asset file for a plugin""" - try: - data = request.get_json() - plugin_id = data.get('plugin_id') - image_id = data.get('image_id') + data = request.get_json() + plugin_id = data.get('plugin_id') + image_id = data.get('image_id') - if not plugin_id or not image_id: - return jsonify({'status': 'error', 'message': 'plugin_id and image_id are required'}), 400 + if not plugin_id or not image_id: + return jsonify({'status': 'error', 'message': 'plugin_id and image_id are required'}), 400 - # Get asset directory - assets_dir = _plugin_uploads_dir(plugin_id) - if assets_dir is None: - return jsonify({'status': 'error', 'message': 'Invalid plugin_id'}), 400 - metadata_file = assets_dir / '.metadata.json' + # Get asset directory + assets_dir = _plugin_uploads_dir(plugin_id) + if assets_dir is None: + return jsonify({'status': 'error', 'message': 'Invalid plugin_id'}), 400 + metadata_file = assets_dir / '.metadata.json' - if not metadata_file.exists(): - return jsonify({'status': 'error', 'message': 'Metadata file not found'}), 404 + if not metadata_file.exists(): + return jsonify({'status': 'error', 'message': 'Metadata file not found'}), 404 - # Load metadata - with open(metadata_file, 'r') as f: - metadata = json.load(f) + # Load metadata + with open(metadata_file, 'r') as f: + metadata = json.load(f) - if image_id not in metadata: - return jsonify({'status': 'error', 'message': 'Image not found'}), 404 + if image_id not in metadata: + return jsonify({'status': 'error', 'message': 'Image not found'}), 404 - # Delete file. The stored path is data, not a trusted location: only - # unlink it when it resolves to a file directly inside this plugin's - # uploads. An entry pointing anywhere else is dropped from the - # metadata without touching the file it names. - entry = metadata[image_id] if isinstance(metadata[image_id], dict) else {} - parts = safe_relative_parts(entry.get('path')) - file_path = resolve_under(PROJECT_ROOT, *parts) if parts else None - if file_path is None or file_path.parent != assets_dir: - logger.warning('Asset %s has a path outside its uploads directory; ' - 'removing the entry without deleting a file', image_id) - elif file_path.exists(): - file_path.unlink() + # Delete file. The stored path is data, not a trusted location: only + # unlink it when it resolves to a file directly inside this plugin's + # uploads. An entry pointing anywhere else is dropped from the + # metadata without touching the file it names. + entry = metadata[image_id] if isinstance(metadata[image_id], dict) else {} + parts = safe_relative_parts(entry.get('path')) + file_path = resolve_under(PROJECT_ROOT, *parts) if parts else None + if file_path is None or file_path.parent != assets_dir: + logger.warning('Asset %s has a path outside its uploads directory; ' + 'removing the entry without deleting a file', image_id) + elif file_path.exists(): + file_path.unlink() - # Remove from metadata - del metadata[image_id] + # Remove from metadata + del metadata[image_id] - # Save metadata - with open(metadata_file, 'w') as f: - json.dump(metadata, f, indent=2) + # Save metadata + with open(metadata_file, 'w') as f: + json.dump(metadata, f, indent=2) - return jsonify({'status': 'success', 'message': 'Image deleted successfully'}) + return jsonify({'status': 'success', 'message': 'Image deleted successfully'}) - except Exception as e: - logger.error('Unhandled exception', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 @api_v3.route('/plugins/assets/list', methods=['GET']) def list_plugin_assets(): """List asset files for a plugin""" - try: - plugin_id = request.args.get('plugin_id') - if not plugin_id: - return jsonify({'status': 'error', 'message': 'plugin_id is required'}), 400 + plugin_id = request.args.get('plugin_id') + if not plugin_id: + return jsonify({'status': 'error', 'message': 'plugin_id is required'}), 400 - # Get asset directory - assets_dir = _plugin_uploads_dir(plugin_id) - if assets_dir is None: - return jsonify({'status': 'error', 'message': 'Invalid plugin_id'}), 400 - metadata_file = assets_dir / '.metadata.json' + # Get asset directory + assets_dir = _plugin_uploads_dir(plugin_id) + if assets_dir is None: + return jsonify({'status': 'error', 'message': 'Invalid plugin_id'}), 400 + metadata_file = assets_dir / '.metadata.json' - if not metadata_file.exists(): - return jsonify({'status': 'success', 'data': {'assets': []}}) + if not metadata_file.exists(): + return jsonify({'status': 'success', 'data': {'assets': []}}) - # Load metadata - with open(metadata_file, 'r') as f: - metadata = json.load(f) + # Load metadata + with open(metadata_file, 'r') as f: + metadata = json.load(f) - # Convert to list - assets = list(metadata.values()) + # Convert to list + assets = list(metadata.values()) - return jsonify({'status': 'success', 'data': {'assets': assets}}) + return jsonify({'status': 'success', 'data': {'assets': assets}}) - except Exception as e: - logger.error('Unhandled exception', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 diff --git a/web_interface/blueprints/api_v3/system.py b/web_interface/blueprints/api_v3/system.py index 1e82b7fd..405c425f 100644 --- a/web_interface/blueprints/api_v3/system.py +++ b/web_interface/blueprints/api_v3/system.py @@ -24,95 +24,91 @@ import web_interface.blueprints.api_v3 as _pkg @api_v3.route('/system/status', methods=['GET']) def get_system_status(): """Get system status""" + # Check cache first (10 second TTL for system status) try: - # Check cache first (10 second TTL for system status) - try: - from web_interface.cache import get_cached, set_cached - cached_result = get_cached('system_status', ttl_seconds=10) - if cached_result is not None: - return jsonify({'status': 'success', 'data': cached_result}) - except ImportError: - # Cache not available, continue without caching - get_cached = None - set_cached = None + from web_interface.cache import get_cached, set_cached + cached_result = get_cached('system_status', ttl_seconds=10) + if cached_result is not None: + return jsonify({'status': 'success', 'data': cached_result}) + except ImportError: + # Cache not available, continue without caching + get_cached = None + set_cached = None - # Import psutil for system monitoring - try: - import psutil - except ImportError: - # Fallback if psutil not available - return jsonify({ - 'status': 'error', - 'message': 'psutil not available for system monitoring' - }), 503 + # Import psutil for system monitoring + try: + import psutil + except ImportError: + # Fallback if psutil not available + return jsonify({ + 'status': 'error', + 'message': 'psutil not available for system monitoring' + }), 503 - # Get system metrics using psutil - cpu_percent = psutil.cpu_percent(interval=0.1) # Short interval for responsiveness - memory = psutil.virtual_memory() - memory_percent = memory.percent - disk = psutil.disk_usage('/') - disk_percent = disk.percent + # Get system metrics using psutil + cpu_percent = psutil.cpu_percent(interval=0.1) # Short interval for responsiveness + memory = psutil.virtual_memory() + memory_percent = memory.percent + disk = psutil.disk_usage('/') + disk_percent = disk.percent - # Calculate uptime - boot_time = psutil.boot_time() - uptime_seconds = _pkg.time.time() - boot_time - uptime_hours = uptime_seconds / 3600 - uptime_days = uptime_hours / 24 + # Calculate uptime + boot_time = psutil.boot_time() + uptime_seconds = _pkg.time.time() - boot_time + uptime_hours = uptime_seconds / 3600 + uptime_days = uptime_hours / 24 - # Format uptime string - if uptime_days >= 1: - uptime_str = f"{int(uptime_days)}d {int(uptime_hours % 24)}h" - elif uptime_hours >= 1: - uptime_str = f"{int(uptime_hours)}h {int((uptime_seconds % 3600) / 60)}m" - else: - uptime_str = f"{int(uptime_seconds / 60)}m" + # Format uptime string + if uptime_days >= 1: + uptime_str = f"{int(uptime_days)}d {int(uptime_hours % 24)}h" + elif uptime_hours >= 1: + uptime_str = f"{int(uptime_hours)}h {int((uptime_seconds % 3600) / 60)}m" + else: + uptime_str = f"{int(uptime_seconds / 60)}m" - # Get CPU temperature (Raspberry Pi) + # Get CPU temperature (Raspberry Pi) + cpu_temp = None + try: + temp_file = '/sys/class/thermal/thermal_zone0/temp' + if os.path.exists(temp_file): + with open(temp_file, 'r') as f: + temp_millidegrees = int(f.read().strip()) + cpu_temp = temp_millidegrees / 1000.0 # Convert to Celsius + except (IOError, ValueError, OSError): + # Temperature sensor not available or error reading cpu_temp = None + + # Get display service status + service_status = _get_display_service_status() + + status = { + 'timestamp': _pkg.time.time(), + 'uptime': uptime_str, + 'uptime_seconds': int(uptime_seconds), + 'service_active': service_status.get('active', False), + 'cpu_percent': round(cpu_percent, 1), + 'memory_used_percent': round(memory_percent, 1), + 'memory_total_mb': round(memory.total / (1024 * 1024), 1), + 'memory_used_mb': round(memory.used / (1024 * 1024), 1), + # MemAvailable, not total-minus-used: it accounts for reclaimable + # page cache, so it is what actually predicts memory trouble. A + # board can read 70% "used" and be fine, or read the same and be + # about to fail fork(), and only this number tells them apart. + 'memory_available_mb': round(memory.available / (1024 * 1024), 1), + 'cpu_temp': round(cpu_temp, 1) if cpu_temp is not None else None, + 'disk_used_percent': round(disk_percent, 1), + 'disk_total_gb': round(disk.total / (1024 * 1024 * 1024), 1), + 'disk_used_gb': round(disk.used / (1024 * 1024 * 1024), 1) + } + + # Cache the result if available + if set_cached: try: - temp_file = '/sys/class/thermal/thermal_zone0/temp' - if os.path.exists(temp_file): - with open(temp_file, 'r') as f: - temp_millidegrees = int(f.read().strip()) - cpu_temp = temp_millidegrees / 1000.0 # Convert to Celsius - except (IOError, ValueError, OSError): - # Temperature sensor not available or error reading - cpu_temp = None + set_cached('system_status', status, ttl_seconds=10) + except Exception: + pass # Cache write failed, but continue - # Get display service status - service_status = _get_display_service_status() - - status = { - 'timestamp': _pkg.time.time(), - 'uptime': uptime_str, - 'uptime_seconds': int(uptime_seconds), - 'service_active': service_status.get('active', False), - 'cpu_percent': round(cpu_percent, 1), - 'memory_used_percent': round(memory_percent, 1), - 'memory_total_mb': round(memory.total / (1024 * 1024), 1), - 'memory_used_mb': round(memory.used / (1024 * 1024), 1), - # MemAvailable, not total-minus-used: it accounts for reclaimable - # page cache, so it is what actually predicts memory trouble. A - # board can read 70% "used" and be fine, or read the same and be - # about to fail fork(), and only this number tells them apart. - 'memory_available_mb': round(memory.available / (1024 * 1024), 1), - 'cpu_temp': round(cpu_temp, 1) if cpu_temp is not None else None, - 'disk_used_percent': round(disk_percent, 1), - 'disk_total_gb': round(disk.total / (1024 * 1024 * 1024), 1), - 'disk_used_gb': round(disk.used / (1024 * 1024 * 1024), 1) - } - - # Cache the result if available - if set_cached: - try: - set_cached('system_status', status, ttl_seconds=10) - except Exception: - pass # Cache write failed, but continue - - return jsonify({'status': 'success', 'data': status}) - except Exception as e: - logger.error('Unhandled exception', exc_info=True) - return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500 + return jsonify({'status': 'success', 'data': status}) @api_v3.route('/system/version', methods=['GET']) def get_system_version(): """Get LEDMatrix repository version""" diff --git a/web_interface/blueprints/api_v3/wifi.py b/web_interface/blueprints/api_v3/wifi.py index 6adf958e..0f03a878 100644 --- a/web_interface/blueprints/api_v3/wifi.py +++ b/web_interface/blueprints/api_v3/wifi.py @@ -111,34 +111,26 @@ def _parse_bool_ish(value): @api_v3.route('/wifi/status', methods=['GET']) def get_wifi_status(): """Get current WiFi connection status""" - try: - from src.wifi_manager import WiFiManager + from src.wifi_manager import WiFiManager - wifi_manager = WiFiManager() - status = wifi_manager.get_wifi_status() + wifi_manager = WiFiManager() + status = wifi_manager.get_wifi_status() - # Get auto-enable setting from config - auto_enable_ap = wifi_manager.config.get("auto_enable_ap_mode", True) # Default: True (safe due to grace period) + # Get auto-enable setting from config + auto_enable_ap = wifi_manager.config.get("auto_enable_ap_mode", True) # Default: True (safe due to grace period) - return jsonify({ - 'status': 'success', - 'data': { - 'connected': status.connected, - 'ssid': status.ssid, - 'ip_address': status.ip_address, - 'signal': status.signal, - 'ap_mode_active': status.ap_mode_active, - 'auto_enable_ap_mode': auto_enable_ap, - 'last_connect_attempt': _last_connect_snapshot(), - } - }) - except Exception as e: - logger.error("%s failed", request.path, exc_info=True) - return jsonify({ - 'status': 'error', - 'message': 'An error occurred; see logs for details', - 'details': describe_exception(e) - }), 500 + return jsonify({ + 'status': 'success', + 'data': { + 'connected': status.connected, + 'ssid': status.ssid, + 'ip_address': status.ip_address, + 'signal': status.signal, + 'ap_mode_active': status.ap_mode_active, + 'auto_enable_ap_mode': auto_enable_ap, + 'last_connect_attempt': _last_connect_snapshot(), + } + }) @api_v3.route('/wifi/scan', methods=['GET']) def scan_wifi_networks(): """Scan for available WiFi networks @@ -219,241 +211,188 @@ def connect_wifi(): background (see _last_connect_attempt); otherwise it waits for the result. """ global _last_connect_attempt - try: - from src.wifi_manager import WiFiManager + from src.wifi_manager import WiFiManager - data = request.get_json(silent=True) - if not data: - return jsonify({ - 'status': 'error', - 'message': 'Request body is required' - }), 400 - - if 'ssid' not in data: - return jsonify({ - 'status': 'error', - 'message': 'SSID is required' - }), 400 - - ssid = data['ssid'] - if not ssid or not ssid.strip(): - return jsonify({ - 'status': 'error', - 'message': 'SSID cannot be empty' - }), 400 - - ssid = ssid.strip() - password = data.get('password', '') or '' - - wifi_manager = WiFiManager() - ap_mode_active = wifi_manager._is_ap_mode_active() - - # One attempt at a time on either path: concurrent connects fight over - # the radio, and the first to finish would clear the in-progress flag - # the monitor daemon still needs for the other. The check can't depend - # on AP state either -- a background attempt takes the AP down long - # before it finishes. - with _connect_lock: - if _last_connect_attempt and _last_connect_attempt['state'] == 'pending': - return jsonify({ - 'status': 'error', - 'message': f"Already connecting to {_last_connect_attempt['ssid']}" - }), 409 - _last_connect_attempt = { - 'ssid': ssid, 'state': 'pending', 'message': None, - 'error_type': None, 'finished_at': None, - } - - if ap_mode_active: - try: - _spawn(lambda: _run_background_connect(ssid, password)) - except Exception: - # Nothing will ever finish this attempt; don't leave every - # later request refused. - with _connect_lock: - _last_connect_attempt = None - raise - return jsonify({ - 'status': 'pending', - 'message': ( - f'Connecting to {ssid}. The LEDMatrix-Setup network will turn off, ' - 'so this page will lose its connection.' - ), - 'data': {'ssid': ssid}, - }), 202 - - try: - success, message = wifi_manager.connect_to_network(ssid, password) - except Exception as e: - _record_connect_result(ssid, {'status': 'error', 'message': describe_exception(e)}) - raise - payload = _connect_result_payload(ssid, success, message) - _record_connect_result(ssid, payload) - return jsonify(payload), (200 if success else 400) - except Exception as e: - logger.error("Error connecting to WiFi", exc_info=True) + data = request.get_json(silent=True) + if not data: return jsonify({ 'status': 'error', - 'message': 'An error occurred; see logs for details', 'details': describe_exception(e) - }), 500 + 'message': 'Request body is required' + }), 400 + + if 'ssid' not in data: + return jsonify({ + 'status': 'error', + 'message': 'SSID is required' + }), 400 + + ssid = data['ssid'] + if not ssid or not ssid.strip(): + return jsonify({ + 'status': 'error', + 'message': 'SSID cannot be empty' + }), 400 + + ssid = ssid.strip() + password = data.get('password', '') or '' + + wifi_manager = WiFiManager() + ap_mode_active = wifi_manager._is_ap_mode_active() + + # One attempt at a time on either path: concurrent connects fight over + # the radio, and the first to finish would clear the in-progress flag + # the monitor daemon still needs for the other. The check can't depend + # on AP state either -- a background attempt takes the AP down long + # before it finishes. + with _connect_lock: + if _last_connect_attempt and _last_connect_attempt['state'] == 'pending': + return jsonify({ + 'status': 'error', + 'message': f"Already connecting to {_last_connect_attempt['ssid']}" + }), 409 + _last_connect_attempt = { + 'ssid': ssid, 'state': 'pending', 'message': None, + 'error_type': None, 'finished_at': None, + } + + if ap_mode_active: + try: + _spawn(lambda: _run_background_connect(ssid, password)) + except Exception: + # Nothing will ever finish this attempt; don't leave every + # later request refused. + with _connect_lock: + _last_connect_attempt = None + raise + return jsonify({ + 'status': 'pending', + 'message': ( + f'Connecting to {ssid}. The LEDMatrix-Setup network will turn off, ' + 'so this page will lose its connection.' + ), + 'data': {'ssid': ssid}, + }), 202 + + try: + success, message = wifi_manager.connect_to_network(ssid, password) + except Exception as e: + _record_connect_result(ssid, {'status': 'error', 'message': describe_exception(e)}) + raise + payload = _connect_result_payload(ssid, success, message) + _record_connect_result(ssid, payload) + return jsonify(payload), (200 if success else 400) @api_v3.route('/wifi/disconnect', methods=['POST']) def disconnect_wifi(): """Disconnect from the current WiFi network""" - try: - from src.wifi_manager import WiFiManager + from src.wifi_manager import WiFiManager - wifi_manager = WiFiManager() - success, message = wifi_manager.disconnect_from_network() + wifi_manager = WiFiManager() + success, message = wifi_manager.disconnect_from_network() - if success: - return jsonify({ - 'status': 'success', - 'message': message - }) - else: - return jsonify({ - 'status': 'error', - 'message': message or 'Failed to disconnect from network' - }), 400 - except Exception as e: - logger.error("Error disconnecting from WiFi", exc_info=True) + if success: + return jsonify({ + 'status': 'success', + 'message': message + }) + else: return jsonify({ 'status': 'error', - 'message': 'An error occurred; see logs for details', 'details': describe_exception(e) - }), 500 + 'message': message or 'Failed to disconnect from network' + }), 400 @api_v3.route('/wifi/ap/enable', methods=['POST']) def enable_ap_mode(): """Enable access point mode""" - try: - from src.wifi_manager import WiFiManager + from src.wifi_manager import WiFiManager - wifi_manager = WiFiManager() - _force_raw = (request.get_json(silent=True) or {}).get('force', False) - force = _force_raw is True or (isinstance(_force_raw, str) and _force_raw.lower() in ('true', '1')) - success, message = wifi_manager.enable_ap_mode(force=force) + wifi_manager = WiFiManager() + _force_raw = (request.get_json(silent=True) or {}).get('force', False) + force = _force_raw is True or (isinstance(_force_raw, str) and _force_raw.lower() in ('true', '1')) + success, message = wifi_manager.enable_ap_mode(force=force) - if success: - return jsonify({ - 'status': 'success', - 'message': message - }) - else: - return jsonify({ - 'status': 'error', - 'message': message - }), 400 - except Exception as e: - logger.error("%s failed", request.path, exc_info=True) + if success: + return jsonify({ + 'status': 'success', + 'message': message + }) + else: return jsonify({ 'status': 'error', - 'message': 'An error occurred; see logs for details', - 'details': describe_exception(e) - }), 500 + 'message': message + }), 400 @api_v3.route('/wifi/ap/disable', methods=['POST']) def disable_ap_mode(): """Disable access point mode""" - try: - from src.wifi_manager import WiFiManager + from src.wifi_manager import WiFiManager - wifi_manager = WiFiManager() - success, message = wifi_manager.disable_ap_mode() + wifi_manager = WiFiManager() + success, message = wifi_manager.disable_ap_mode() - if success: - return jsonify({ - 'status': 'success', - 'message': message - }) - else: - return jsonify({ - 'status': 'error', - 'message': message - }), 400 - except Exception as e: - logger.error("%s failed", request.path, exc_info=True) + if success: + return jsonify({ + 'status': 'success', + 'message': message + }) + else: return jsonify({ 'status': 'error', - 'message': 'An error occurred; see logs for details', - 'details': describe_exception(e) - }), 500 + 'message': message + }), 400 @api_v3.route('/wifi/ap/auto-enable', methods=['GET']) def get_auto_enable_ap_mode(): """Get auto-enable AP mode setting""" - try: - from src.wifi_manager import WiFiManager + from src.wifi_manager import WiFiManager - wifi_manager = WiFiManager() - auto_enable = wifi_manager.config.get("auto_enable_ap_mode", True) # Default: True (safe due to grace period) + wifi_manager = WiFiManager() + auto_enable = wifi_manager.config.get("auto_enable_ap_mode", True) # Default: True (safe due to grace period) - return jsonify({ - 'status': 'success', - 'data': { - 'auto_enable_ap_mode': auto_enable - } - }) - except Exception as e: - logger.error("%s failed", request.path, exc_info=True) - return jsonify({ - 'status': 'error', - 'message': 'An error occurred; see logs for details', - 'details': describe_exception(e) - }), 500 + return jsonify({ + 'status': 'success', + 'data': { + 'auto_enable_ap_mode': auto_enable + } + }) @api_v3.route('/wifi/ap/auto-enable', methods=['POST']) def set_auto_enable_ap_mode(): """Set auto-enable AP mode setting""" - try: - from src.wifi_manager import WiFiManager + from src.wifi_manager import WiFiManager - data = request.get_json(silent=True) - if data is None or 'auto_enable_ap_mode' not in data: - return jsonify({ - 'status': 'error', - 'message': 'auto_enable_ap_mode is required' - }), 400 - - auto_enable = _parse_bool_ish(data['auto_enable_ap_mode']) - if auto_enable is None: - return jsonify({ - 'status': 'error', - 'message': 'auto_enable_ap_mode must be a boolean' - }), 400 - - wifi_manager = WiFiManager() - wifi_manager.config["auto_enable_ap_mode"] = auto_enable - wifi_manager._save_config() - - return jsonify({ - 'status': 'success', - 'message': f'Auto-enable AP mode set to {auto_enable}', - 'data': { - 'auto_enable_ap_mode': auto_enable - } - }) - except Exception as e: - logger.error("%s failed", request.path, exc_info=True) + data = request.get_json(silent=True) + if data is None or 'auto_enable_ap_mode' not in data: return jsonify({ 'status': 'error', - 'message': 'An error occurred; see logs for details', - 'details': describe_exception(e) - }), 500 + 'message': 'auto_enable_ap_mode is required' + }), 400 + + auto_enable = _parse_bool_ish(data['auto_enable_ap_mode']) + if auto_enable is None: + return jsonify({ + 'status': 'error', + 'message': 'auto_enable_ap_mode must be a boolean' + }), 400 + + wifi_manager = WiFiManager() + wifi_manager.config["auto_enable_ap_mode"] = auto_enable + wifi_manager._save_config() + + return jsonify({ + 'status': 'success', + 'message': f'Auto-enable AP mode set to {auto_enable}', + 'data': { + 'auto_enable_ap_mode': auto_enable + } + }) @api_v3.route('/wifi/radio', methods=['GET']) def get_wifi_radio(): """Get current WiFi radio state (enabled/disabled) and wired-fallback status.""" - try: - from src.wifi_manager import WiFiManager + from src.wifi_manager import WiFiManager - wifi_manager = WiFiManager() - state = wifi_manager.get_wifi_radio_state() + wifi_manager = WiFiManager() + state = wifi_manager.get_wifi_radio_state() - return jsonify({ - 'status': 'success', - 'data': state - }) - except Exception as e: - logger.error("Error getting WiFi radio state", exc_info=True) - return jsonify({ - 'status': 'error', - 'message': 'An error occurred; see logs for details', 'details': describe_exception(e) - }), 500 + return jsonify({ + 'status': 'success', + 'data': state + }) @api_v3.route('/wifi/radio', methods=['POST']) def set_wifi_radio(): """Turn the WiFi radio on or off. @@ -462,53 +401,46 @@ def set_wifi_radio(): unless Ethernet is connected or force=True, to avoid locking the user out of this web interface. """ - try: - from src.wifi_manager import WiFiManager + from src.wifi_manager import WiFiManager - data = request.get_json(silent=True) or {} - if 'enabled' not in data: - return jsonify({ - 'status': 'error', - 'message': 'enabled is required' - }), 400 - - # Parse defensively: bool("false") is True and a plain int never - # matches `is True`, so `_parse_bool_ish` handles bool, string and - # int 1/0 — the endpoint is a public contract, not just the shipped - # UI (which always sends real JSON booleans). An unrecognized value - # must be rejected, not silently disable the radio: this is the - # route that can drop the caller's own connection to this interface. - enabled = _parse_bool_ish(data['enabled']) - if enabled is None: - return jsonify({ - 'status': 'error', - 'message': 'enabled must be a boolean' - }), 400 - force = _parse_bool_ish(data.get('force', False)) - if force is None: - return jsonify({ - 'status': 'error', - 'message': 'force must be a boolean' - }), 400 - - wifi_manager = WiFiManager() - success, message, reason = wifi_manager.set_wifi_radio(enabled, force=force) - - if success: - return jsonify({ - 'status': 'success', - 'message': message, - 'data': wifi_manager.get_wifi_radio_state() - }) - else: - return jsonify({ - 'status': 'error', - 'message': message, - 'reason': reason - }), 400 - except Exception as e: - logger.error("Error setting WiFi radio state", exc_info=True) + data = request.get_json(silent=True) or {} + if 'enabled' not in data: return jsonify({ 'status': 'error', - 'message': 'An error occurred; see logs for details', 'details': describe_exception(e) - }), 500 + 'message': 'enabled is required' + }), 400 + + # Parse defensively: bool("false") is True and a plain int never + # matches `is True`, so `_parse_bool_ish` handles bool, string and + # int 1/0 — the endpoint is a public contract, not just the shipped + # UI (which always sends real JSON booleans). An unrecognized value + # must be rejected, not silently disable the radio: this is the + # route that can drop the caller's own connection to this interface. + enabled = _parse_bool_ish(data['enabled']) + if enabled is None: + return jsonify({ + 'status': 'error', + 'message': 'enabled must be a boolean' + }), 400 + force = _parse_bool_ish(data.get('force', False)) + if force is None: + return jsonify({ + 'status': 'error', + 'message': 'force must be a boolean' + }), 400 + + wifi_manager = WiFiManager() + success, message, reason = wifi_manager.set_wifi_radio(enabled, force=force) + + if success: + return jsonify({ + 'status': 'success', + 'message': message, + 'data': wifi_manager.get_wifi_radio_state() + }) + else: + return jsonify({ + 'status': 'error', + 'message': message, + 'reason': reason + }), 400