Merge pull request #2291 from AnishSarkar22/hotpatch/plugin-calls

fix(local): tunnels through SurfSense's proxy work under uvloop

docs/architecture/agent.md

@@ -247,7 +247,7 @@ The calls, their arguments and results are kept in the thread's opencode session
 
 ## Network
 
-opencode needs no host beyond loopback but a ready plugin's: its model is the model endpoint, its tools are its own and SurfSense's, on the API's port, and a plugin's server is reached through the API's plugin proxy. Its environment is built from a few system variables rather than inherited. It sets `OPENCODE_DISABLE_MODELS_FETCH`, `OPENCODE_DISABLE_SHARE`, `OPENCODE_DISABLE_LSP_DOWNLOAD`, `OPENCODE_DISABLE_AUTOUPDATE`, `OPENCODE_DISABLE_DEFAULT_PLUGINS`, `OPENCODE_PURE`, `OPENCODE_DISABLE_PROJECT_CONFIG`, `OPENCODE_DISABLE_EXTERNAL_SKILLS` and `OPENCODE_DISABLE_CLAUDE_CODE`. Its proxy variables point at the plugin proxy, with loopback exempted. Electron picks its port beside opencode's and gives it to the API as `SURFSENSE_LOCAL_PLUGIN_PROXY_PORT`, and the API runs it while it runs ([`plugin_proxy/`](../../surfsense_local/backend/modules/agent/plugin_proxy/)). It passes only `CONNECT` to a ready plugin's host, on its URL's port or 443 for a host its entry declares; anything else gets `403`. The ready hosts are kept in memory, so a connection takes no database write lock, and read again once a commit changes a row readiness reads, and at least every 2 seconds, since a registry's delisting is a file. A host passed is recorded as that host's last call at most once a minute, off the connection's path. A host that does not accept within 10 seconds, or a check that fails, is answered `502` rather than closed unanswered. A fetch this misses therefore fails rather than leaving the machine, as it did when the variables pointed at `127.0.0.1:9`, where they still point when no port was given. The tunnel carries opencode's TLS untouched, so SurfSense sees the host and never the content, and a remote plugin must be on HTTPS: one added by an `http://` URL off this computer is refused when connected. The npm install opencode would otherwise run at startup is skipped: its config folder holds an empty `node_modules` and a lockfile naming the package. ripgrep is the shipped one, first on its `PATH`. Settings › Network gains nothing: opencode is never a destination, a remote model's host is checked as its connection's, and a plugin's as the plugin's.
+opencode needs no host beyond loopback but a ready plugin's: its model is the model endpoint, its tools are its own and SurfSense's, on the API's port, and a plugin's server is reached through the API's plugin proxy. Its environment is built from a few system variables rather than inherited. It sets `OPENCODE_DISABLE_MODELS_FETCH`, `OPENCODE_DISABLE_SHARE`, `OPENCODE_DISABLE_LSP_DOWNLOAD`, `OPENCODE_DISABLE_AUTOUPDATE`, `OPENCODE_DISABLE_DEFAULT_PLUGINS`, `OPENCODE_PURE`, `OPENCODE_DISABLE_PROJECT_CONFIG`, `OPENCODE_DISABLE_EXTERNAL_SKILLS` and `OPENCODE_DISABLE_CLAUDE_CODE`. Its proxy variables point at the plugin proxy, with loopback exempted. Electron picks its port beside opencode's and gives it to the API as `SURFSENSE_LOCAL_PLUGIN_PROXY_PORT`, and the API runs it while it runs ([`plugin_proxy/`](../../surfsense_local/backend/modules/agent/plugin_proxy/)). It passes only `CONNECT` to a ready plugin's host, on its URL's port or 443 for a host its entry declares; anything else gets `403`. The ready hosts are kept in memory, so a connection takes no database write lock, and read again once a commit changes a row readiness reads, and at least every 2 seconds, since a registry's delisting is a file. A host passed is recorded as that host's last call at most once a minute, off the connection's path. A host that does not accept within 10 seconds, a connect that fails in any other way, or a check that fails, is answered `502` rather than closed unanswered, which a client would retry blindly. On asyncio's own loop, which the API runs on Windows and in tests, a host's next address is tried 0.25 s after the last; uvloop, which it runs on macOS and Linux, has no such option, so there the connect takes the default. A fetch this misses therefore fails rather than leaving the machine, as it did when the variables pointed at `127.0.0.1:9`, where they still point when no port was given. The tunnel carries opencode's TLS untouched, so SurfSense sees the host and never the content, and a remote plugin must be on HTTPS: one added by an `http://` URL off this computer is refused when connected. The npm install opencode would otherwise run at startup is skipped: its config folder holds an empty `node_modules` and a lockfile naming the package. ripgrep is the shipped one, first on its `PATH`. Settings › Network gains nothing: opencode is never a destination, a remote model's host is checked as its connection's, and a plugin's as the plugin's.
 
 A document script is another matter: it runs in the Studio worker's runner, which gives it an environment of a few system variables and no secrets but does not stop it opening connections ([ADR 0039](../adr/0039-document-scripts-run-without-approval.md), [studio](studio.md#script-documents)).
 

surfsense_local/backend/modules/agent/plugin_proxy/server.py

@@ -60,6 +60,18 @@ async def serving_plugin_proxy(
         yield server.sockets[0].getsockname()[1]
 
 
+async def _connect(
+    host: str, port: int
+) -> tuple[asyncio.StreamReader, asyncio.StreamWriter]:
+    """asyncio's own loop takes the next address after a delay; uvloop, which the
+    API runs on macOS and Linux, has no such option and refuses the call."""
+    if isinstance(asyncio.get_running_loop(), asyncio.BaseEventLoop):
+        return await asyncio.open_connection(
+            host, port, happy_eyeballs_delay=_NEXT_ADDRESS_SECONDS
+        )
+    return await asyncio.open_connection(host, port)
+
+
 async def _tunnel(
     allowed: AllowedHosts,
     recording: set[asyncio.Task[None]],
@@ -93,12 +105,11 @@ async def _tunnel(
         return
     try:
         upstream_reader, upstream_writer = await asyncio.wait_for(
-            asyncio.open_connection(
-                host, int(port), happy_eyeballs_delay=_NEXT_ADDRESS_SECONDS
-            ),
-            CONNECT_SECONDS,
+            _connect(host, int(port)), CONNECT_SECONDS
         )
-    except (OSError, TimeoutError):
+    # Any failure, not only the network's: a socket closed unanswered reads as a
+    # blip, and opencode marks the plugin failed without saying why.
+    except Exception:
         logger.warning("could not reach %s for opencode", target, exc_info=True)
         await _answer(writer, b"502 Bad Gateway")
         return

surfsense_local/backend/tests/integration/agent/test_plugin_proxy.py

@@ -11,12 +11,13 @@ from urllib.parse import urlsplit
 
 import pytest
 import pytest_asyncio
+import uvloop
 from sqlalchemy import Engine
 from sqlalchemy.exc import OperationalError
 from sqlalchemy.orm import Session
 
 from api.main import create_app, lifespan
-from modules.agent.plugin_proxy import allowed_hosts
+from modules.agent.plugin_proxy import allowed_hosts, server
 from modules.agent.plugin_proxy.allowed_hosts import AllowedHosts
 from modules.agent.plugin_proxy.server import serving_plugin_proxy
 from modules.egress.models import EgressDestination
@@ -281,6 +282,24 @@ async def test_a_host_that_cannot_be_reached_is_answered(
     assert head.startswith(b"HTTP/1.1 502")
 
 
+async def test_a_connect_that_fails_in_any_way_is_answered(
+    engine: Engine, proxy: int, echo: Echo, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Not only a network error: a dropped socket reads as a blip, and is retried blindly."""
+    connect_plugin(engine, f"https://localhost:{echo.port}/mcp")
+
+    async def unexpected(_host: str, _port: int) -> None:
+        raise RuntimeError("the loop refused the call")
+
+    monkeypatch.setattr(server, "_connect", unexpected)
+
+    head, _ = await through(
+        proxy, f"CONNECT localhost:{echo.port} HTTP/1.1\r\n\r\n".encode()
+    )
+
+    assert head.startswith(b"HTTP/1.1 502")
+
+
 async def test_a_check_that_fails_is_answered(
     engine: Engine, proxy: int, echo: Echo, monkeypatch: pytest.MonkeyPatch
 ) -> None:
@@ -433,3 +452,33 @@ async def test_the_api_runs_the_proxy_where_electron_points_opencode(
         head, _ = await through(port, b"CONNECT example.com:443 HTTP/1.1\r\n\r\n")
 
     assert head.startswith(b"HTTP/1.1 403")
+
+
+def test_a_ready_plugins_host_is_tunnelled_to_under_uvloop(engine: Engine) -> None:
+    """The real app runs on uvloop on macOS and Linux; the tests' loop is asyncio's."""
+
+    async def tunnelled() -> tuple[bytes, bytes]:
+        async def answer(
+            reader: asyncio.StreamReader, writer: asyncio.StreamWriter
+        ) -> None:
+            writer.write(await reader.read(100))
+            await writer.drain()
+            writer.close()
+
+        echo = await asyncio.start_server(answer, "127.0.0.1", 0)
+        port = echo.sockets[0].getsockname()[1]
+        connect_plugin(engine, f"https://localhost:{port}/mcp")
+        async with (
+            echo,
+            serving_plugin_proxy(
+                create_session_factory(engine), "127.0.0.1", 0
+            ) as proxy,
+        ):
+            return await through(
+                proxy, f"CONNECT localhost:{port} HTTP/1.1\r\n\r\n".encode()
+            )
+
+    head, echoed = uvloop.run(tunnelled())
+
+    assert head.startswith(b"HTTP/1.1 200")
+    assert echoed == b"ping"