diff --git a/api/controllers/openapi/__init__.py b/api/controllers/openapi/__init__.py index 4bd436418f..872493f3c8 100644 --- a/api/controllers/openapi/__init__.py +++ b/api/controllers/openapi/__init__.py @@ -1,6 +1,7 @@ from flask import Blueprint from flask_restx import Namespace +from controllers.openapi._errors import OpenApiErrorFormatter from libs.device_flow_security import attach_anti_framing from libs.external_api import ExternalApi @@ -12,6 +13,7 @@ api = ExternalApi( version="1.0", title="OpenAPI", description="User-scoped programmatic API (bearer auth)", + error_body_formatter=OpenApiErrorFormatter(), ) openapi_ns = Namespace("openapi", description="User-scoped operations", path="/") diff --git a/api/libs/external_api.py b/api/libs/external_api.py index 378cceff6b..077edad6b7 100644 --- a/api/libs/external_api.py +++ b/api/libs/external_api.py @@ -2,7 +2,7 @@ import re from collections.abc import Mapping from typing import Any, Protocol -from flask import Blueprint, Flask, current_app, got_request_exception +from flask import Blueprint, Flask, current_app, got_request_exception, request from flask_restx import Api from werkzeug.exceptions import HTTPException from werkzeug.http import HTTP_STATUS_CODES @@ -133,13 +133,43 @@ class ExternalApi(Api): } def __init__(self, app: Blueprint | Flask, *args, error_body_formatter: ErrorBodyFormatter | None = None, **kwargs): + self._error_body_formatter = error_body_formatter patch_swagger_for_inline_nested_dicts() kwargs.setdefault("authorizations", self._authorizations) kwargs.setdefault("security", "Bearer") kwargs["add_specs"] = dify_config.SWAGGER_UI_ENABLED kwargs["doc"] = dify_config.SWAGGER_UI_PATH if dify_config.SWAGGER_UI_ENABLED else False + if error_body_formatter is not None: + kwargs.setdefault("catch_all_404s", True) + # the overrides below patch private flask-restx methods; fail at + # startup (not at the first 404) if an upgrade removes them + for private_hook in ("_should_use_fr_error_handler", "_help_on_404"): + if not callable(getattr(Api, private_hook, None)): + raise RuntimeError(f"flask-restx no longer exposes {private_hook}; update ExternalApi overrides") # manual separate call on construction and init_app to ensure configs in kwargs effective super().__init__(app=None, *args, **kwargs) self.init_app(app, **kwargs) register_external_error_handlers(self, body_formatter=error_body_formatter) + + def _should_use_fr_error_handler(self): + # catch_all_404s makes flask-restx claim NotFound for ANY app path + # (it wraps the app-level handle_exception), so scope the claim to + # this blueprint's url prefix; other surfaces keep their own 404s. + if self._error_body_formatter is not None and not self._request_under_own_prefix(): + return False + return super()._should_use_fr_error_handler() + + def _request_under_own_prefix(self) -> bool: + prefix = self.blueprint.url_prefix if self.blueprint is not None else None + if not prefix: + return True + return request.path == prefix or request.path.startswith(prefix.rstrip("/") + "/") + + def _help_on_404(self, message: str | None = None) -> str | None: + # flask-restx appends route suggestions post-handler; with a canonical + # formatter installed, that would corrupt the contract and enumerate + # routes to unauthenticated callers. + if self._error_body_formatter is not None: + return message + return super()._help_on_404(message) diff --git a/api/tests/unit_tests/controllers/openapi/test_error_contract.py b/api/tests/unit_tests/controllers/openapi/test_error_contract.py index d559385d10..3165ac405c 100644 --- a/api/tests/unit_tests/controllers/openapi/test_error_contract.py +++ b/api/tests/unit_tests/controllers/openapi/test_error_contract.py @@ -1,5 +1,7 @@ """Wire-contract tests for the canonical /openapi/v1 error body.""" +from unittest.mock import MagicMock, patch + import pytest from werkzeug.exceptions import Conflict, NotFound, UnprocessableEntity @@ -173,3 +175,59 @@ class TestOpenApiErrorFormatter: for e, data, status in cases: wire = fmt.finalize(e, data, status) assert wire["code"] in {c.value for c in OpenApiErrorCode} + + +class TestWireContract: + """End-to-end: request in, canonical JSON out, through the real openapi blueprint.""" + + def test_accepts_422_carries_code_status_details(self, openapi_app, bypass_pipeline): + client = openapi_app.test_client() + + resp = client.get("/openapi/v1/apps?page=0") + + assert resp.status_code == 422 + wire = resp.get_json() + ErrorBody.model_validate(wire) + assert wire["code"] == "invalid_param" + assert wire["status"] == 422 + assert wire["details"] + + def test_unknown_route_404_is_canonical_without_route_suggestions(self, openapi_app): + client = openapi_app.test_client() + + resp = client.get("/openapi/v1/definitely-not-a-route") + + assert resp.status_code == 404 + wire = resp.get_json() + ErrorBody.model_validate(wire) + assert wire["code"] == "not_found" + assert "did you mean" not in wire["message"].lower() + + def test_404_outside_blueprint_prefix_is_not_claimed(self, openapi_app): + # catch_all_404s wraps the app-level exception handler; the prefix + # guard must keep non-/openapi/v1 paths on the app's own 404 handling + client = openapi_app.test_client() + + resp = client.get("/console/definitely-not-a-route") + + assert resp.status_code == 404 + # not intercepted → Flask's default HTML 404, not the canonical JSON body + assert "application/json" not in (resp.content_type or "") + + @patch("controllers.openapi.oauth_device.DeviceFlowRedis") + def test_oauth_device_token_keeps_rfc8628_shape(self, mock_redis_cls, openapi_app): + store = MagicMock() + mock_redis_cls.return_value = store + store.record_poll.return_value = None # not SlowDownDecision.SLOW_DOWN + store.load_by_device_code.return_value = None # unknown code → expired_token + + client = openapi_app.test_client() + + resp = client.post( + "/openapi/v1/oauth/device/token", + json={"client_id": "difyctl", "device_code": "nope"}, + ) + + assert resp.status_code == 400 + wire = resp.get_json() + assert wire == {"error": "expired_token"}