diff --git a/app/api_docs.py b/app/api_docs.py index ee9247624..48ecd50de 100644 --- a/app/api_docs.py +++ b/app/api_docs.py @@ -1,111 +1,59 @@ -"""Browsable API documentation for the ComfyUI HTTP API. +"""Serves openapi.yaml and renders it as browsable API docs. -Renders the repository's ``openapi.yaml`` with Redoc. Redoc is used rather than -Swagger UI because it has no request-execution feature at all: the local server -is unauthenticated by default, so a docs page that could fire requests would -give one-click access to destructive endpoints such as ``/api/interrupt``, -``/api/free`` and ``DELETE /api/userdata/{file}``. - -The viewer bundle is loaded from a CDN, so the page needs outbound network -access to render. Installs without it still get the raw spec from -``/openapi.yaml``; the fallback notice below points there. +Uses Redoc, not Swagger UI: the local server is unauthenticated by default, so +the viewer must not be able to execute requests. """ import os from aiohttp import web -# openapi.yaml lives next to server.py at the repository root. Resolve it from -# __file__ rather than the cwd: ComfyUI is routinely launched from other -# directories and through wrappers, and a relative path would 404 unpredictably. SPEC_PATH = os.path.join( os.path.dirname(os.path.dirname(os.path.realpath(__file__))), "openapi.yaml" ) -# Pinned to an exact version rather than a floating tag so a CDN-side release -# cannot change what this page executes. -# -# TODO: add an integrity="sha384-..." attribute. The pinned path is immutable, -# so SRI is worth having; the hash simply could not be computed where this was -# written (no outbound network), and a wrong hash fails the page closed. -REDOC_BUNDLE_URL = "https://cdn.jsdelivr.net/npm/redoc@2.5.0/bundles/redoc.standalone.js" - -# The spec URL is deliberately relative. add_routes() re-registers every route -# under an /api prefix, so this page is reachable at both /api-docs and -# /api/api-docs; a relative URL resolves to the sibling spec in either case. -SPEC_URL = "openapi.yaml" - -# Substituted with str.replace rather than str.format/f-string so the CSS braces -# below stay literal and a future style edit does not have to double them. +# The spec URL is relative so it resolves from both /api-docs and /api/api-docs. +# The viewer is pinned and loaded from a CDN, so the page needs network access; +# if it fails to load, the fallback below points at the locally served spec. API_DOCS_HTML = """ ComfyUI API Reference - - - -
-

API docs viewer unavailable

-

- The documentation viewer is loaded from a CDN and could not be reached. - This is expected on an offline or air-gapped install. -

-

- The specification itself is served locally and needs no network access: - openapi.yaml. Render it with any local viewer, - for example npx @redocly/cli preview-docs openapi.yaml. -

+ + + -""".replace("__SPEC_URL__", SPEC_URL).replace("__BUNDLE_URL__", REDOC_BUNDLE_URL) +""" def add_api_docs_routes(routes: web.RouteTableDef) -> None: - """Register the spec and docs-page routes on the given route table. - - Registering on PromptServer's route table (rather than on the app directly) - matters twice over: the table is added before the ``web.static('/')`` - catch-all that would otherwise shadow these paths, and every route in it is - also re-registered under an ``/api`` prefix. - """ + """Register /openapi.yaml and /api-docs on the given route table.""" @routes.get("/openapi.yaml") async def get_openapi_spec(request): - if not os.path.isfile(SPEC_PATH): - return web.Response(status=404, text="openapi.yaml not found") - response = web.FileResponse(SPEC_PATH) - response.headers["Content-Type"] = "application/yaml" - # The cache_control middleware only special-cases js/css/images, so a - # .yaml response falls through untouched. Without this, a user editing - # the spec would keep getting a stale copy from the browser cache. - response.headers["Cache-Control"] = "no-store, must-revalidate" - return response + # The spec is edited in place during development, so never let the + # browser hold a stale copy. no-cache still allows a 304 via the ETag + # FileResponse sets, which matters for a ~230 KB file. + return web.FileResponse(SPEC_PATH, headers={ + "Content-Type": "application/yaml", + "Cache-Control": "no-cache", + }) @routes.get("/api-docs") async def get_api_docs(request): - response = web.Response(text=API_DOCS_HTML, content_type="text/html") - response.headers["Cache-Control"] = "no-store, must-revalidate" - return response + return web.Response( + text=API_DOCS_HTML, + content_type="text/html", + headers={"Cache-Control": "no-cache"}, + ) diff --git a/tests-unit/server_test/test_api_docs.py b/tests-unit/server_test/test_api_docs.py index 42d7f0dd6..2b8940860 100644 --- a/tests-unit/server_test/test_api_docs.py +++ b/tests-unit/server_test/test_api_docs.py @@ -1,143 +1,69 @@ """Tests for the OpenAPI spec and API docs routes.""" -import os - import pytest import pytest_asyncio import yaml from aiohttp import web -from app.api_docs import SPEC_PATH, SPEC_URL, add_api_docs_routes +from app.api_docs import add_api_docs_routes pytestmark = pytest.mark.asyncio -def _build_app(): - """Mirror how PromptServer mounts these routes, including the /api prefix. - - add_routes() walks the route table and re-registers every RouteDef under an - /api prefix, so both the bare and prefixed forms are served. Reproducing - that here keeps the prefix behaviour covered by tests. - """ +@pytest_asyncio.fixture +async def client(aiohttp_client): app = web.Application() routes = web.RouteTableDef() add_api_docs_routes(routes) - - api_routes = web.RouteTableDef() - for route in routes: - if isinstance(route, web.RouteDef): - api_routes.route(route.method, "/api" + route.path)( - route.handler, **route.kwargs - ) - app.add_routes(api_routes) app.add_routes(routes) - return app + return await aiohttp_client(app) -@pytest_asyncio.fixture -async def client(aiohttp_client): - return await aiohttp_client(_build_app()) - - -async def test_spec_path_points_at_the_repo_spec(): - """SPEC_PATH must resolve from __file__, not the cwd.""" - assert os.path.isfile(SPEC_PATH) - assert os.path.basename(SPEC_PATH) == "openapi.yaml" - - -async def test_get_spec_returns_yaml(client): +async def test_get_spec(client): resp = await client.get("/openapi.yaml") assert resp.status == 200 assert resp.headers["Content-Type"] == "application/yaml" - assert resp.headers["Cache-Control"] == "no-store, must-revalidate" - - -async def test_spec_body_is_valid_openapi_3(client): - resp = await client.get("/openapi.yaml") + assert resp.headers["Cache-Control"] == "no-cache" spec = yaml.safe_load(await resp.text()) assert spec["openapi"].startswith("3.") assert spec["paths"] -async def test_spec_is_also_served_under_the_api_prefix(client): - """Documents the prefix duplication so a refactor cannot silently break it.""" - resp = await client.get("/api/openapi.yaml") - assert resp.status == 200 - assert resp.headers["Content-Type"] == "application/yaml" - - async def test_missing_spec_returns_404(client, monkeypatch): monkeypatch.setattr("app.api_docs.SPEC_PATH", "/nonexistent/openapi.yaml") resp = await client.get("/openapi.yaml") assert resp.status == 404 -async def test_docs_page_returns_html_referencing_the_spec(client): +async def test_docs_page(client): resp = await client.get("/api-docs") assert resp.status == 200 assert resp.content_type == "text/html" body = await resp.text() - assert SPEC_URL in body - - -async def test_docs_page_spec_url_is_relative(client): - """A relative URL resolves correctly from both /api-docs and /api/api-docs.""" - assert not SPEC_URL.startswith("/") - resp = await client.get("/api/api-docs") - assert resp.status == 200 - - -async def test_docs_page_cannot_execute_requests(client): - """The local server is unauthenticated, so the docs UI must not fire requests. - - Redoc has no request execution at all. Guard against a swap to Swagger UI, - whose "Try it out" would give one-click access to destructive endpoints. - """ - body = (await (await client.get("/api-docs")).text()).lower() + # Redoc has no request execution; Swagger UI's "Try it out" would give + # one-click access to destructive endpoints on an unauthenticated server. assert "' in body + assert 'spec-url="openapi.yaml"' in body -async def test_routes_survive_the_static_catch_all(aiohttp_client, tmp_path): - """web.static('/') is registered last and matches everything. - - Registering on PromptServer's route table is what keeps these paths - reachable; this fails if they are ever moved after the catch-all. - """ - (tmp_path / "index.html").write_text("frontend") - (tmp_path / "docs").mkdir() - (tmp_path / "docs" / "node.json").write_text("{}") - - app = _build_app() - app.add_routes([web.static("/docs", tmp_path / "docs")]) - app.add_routes([web.static("/", tmp_path)]) - client = await aiohttp_client(app) - - assert (await client.get("/openapi.yaml")).headers["Content-Type"] == ( - "application/yaml" - ) - assert (await client.get("/api-docs")).content_type == "text/html" - # Embedded node docs and the frontend bundle are untouched. - assert await (await client.get("/docs/node.json")).text() == "{}" - assert await (await client.get("/index.html")).text() == "frontend" - - -async def test_api_docs_flag_is_off_by_default(): - """Both routes are gated on this flag in PromptServer.add_routes().""" +async def test_flag_is_off_by_default(): + """Serving the API surface of an unauthenticated server is opt-in.""" from comfy.cli_args import parser assert parser.parse_args([]).enable_api_docs is False