feat(orthrus): server-side Docker proxy listener for agent sessions#1027
Conversation
defer conn.Close() and stream.Close() in proxyConn are cleanup-only deferred calls where the error is not actionable; use _ = x.Close() idiom to satisfy golangci-lint errcheck rule
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
✅ Supply Chain Verification Results✅ PASSED 📦 SBOM Summary
🔍 Vulnerability Scan
📎 Artifacts
Generated by Supply Chain Verification workflow • View Details |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
… Orthrus UUID - Add TestAgentSession_ProxyConn_OpenFails: exercises the early-return path inside proxyConn when the underlying yamux session is already closed, causing Open() to return ErrSessionShutdown immediately - Add TestAgentSession_ProxyConn_WriteFails: exercises the early-return path when stream.Write() fails; the test goroutine consumes the 12-byte yamux SYN frame from the pipe (allowing Open to succeed) then closes the connection so the subsequent DATA frame write returns an error - Add TestDockerHandler_ListContainers_OrthrusEmptyAgentUUID: exercises the right-hand branch of the OrthrusAgentUUID guard (non-nil pointer to empty string), covering the 400 response path that was previously only half-covered by the nil-pointer test Together these tests bring Codecov patch coverage for the Orthrus feature branch from 83% toward the 90% threshold required for merge Closes coverage gap on PR #1027
There was a problem hiding this comment.
Pull request overview
Implements the server-side half of the Orthrus agent Docker tunnel: when an agent WebSocket connects, the server allocates an ephemeral loopback TCP listener that proxies each TCP connection into a fresh yamux stream prefixed with the 0x01 Docker stream-type byte. The Docker handler now routes connection_type == "orthrus" remote servers through that loopback address instead of tcp://host:port, with explicit 400/502/503 errors for misconfiguration, offline agents, and disabled subsystems.
Changes:
- Add
StartDockerProxy/runProxyListener/proxyConn/listener lifecycle toAgentSessionand wire the listener start/stop into the WebSocket connect and heartbeat-watcher paths. - Add
orthrusProxyResolverinterface +SetOrthrusResolvertoDockerHandlerand branch onConnectionTypewhen resolving the Docker host; hoistorthrusServerinroutes.goto wire it. - Add unit tests, an integration-build stub, an extensive feature spec, and a QA/security audit report; minor unrelated test-file whitespace/struct-alignment cleanups.
Reviewed changes
Copilot reviewed 18 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| backend/internal/orthrus/session.go | Adds streamTypeDocker, listener field, StartDockerProxy, runProxyListener, proxyConn; updates Close/GetProxyAddr sentinel |
| backend/internal/orthrus/session_test.go | New tests covering loopback bind, idempotency, close-stops-listener, and type-byte write |
| backend/internal/orthrus/session_coverage_test.go | Extra coverage for proxyConn Open/Write failure paths; updates existing test for new sentinel |
| backend/internal/orthrus/server.go | Calls StartDockerProxy on connect; calls sess.Close() in heartbeat-watcher cleanup |
| backend/internal/orthrus/server_test.go | New end-to-end WebSocket test asserting a loopback proxy address is registered |
| backend/internal/orthrus/server_coverage_test.go | Updates existing GetProxyAddr test for new listener sentinel |
| backend/internal/orthrus/proxy_integration_test.go | New //go:build integration skip-stub |
| backend/internal/api/handlers/docker_handler.go | Adds orthrusProxyResolver interface, setter, and ConnectionType switch with 400/502/503 branches |
| backend/internal/api/handlers/docker_handler_test.go | New tests for connected/offline/no-resolver/nil-UUID/empty-UUID and nil setter cases |
| backend/internal/api/routes/routes.go | Hoists orthrusServer declaration and wires SetOrthrusResolver |
| backend/internal/config/config.go | Re-aligns struct fields (adds CertExpiryWarningDays) |
| backend/internal/api/handlers/crowdsec_stop_lapi_test.go | Struct-field alignment formatting |
| backend/internal/api/handlers/crowdsec_coverage_target_test.go | Whitespace/indent cleanup |
| backend/internal/api/handlers/crowdsec_wave5_test.go, security_notifications_single_source_test.go, system_permissions_*_test.go, certificate_validator_extra_coverage_test.go | Blank-line whitespace cleanups only |
| SECURITY.md | Updates Last Updated date |
| docs/reports/qa-security-audit-2026-05-18.md | New QA/security audit report |
| docs/plans/current_spec.md | Replaces archived CI-fix spec with the Orthrus Docker proxy feature spec |
…nter Unconditionally passing orthrusServer (a nil *OrthrusServer) to SetOrthrusResolver stored a non-nil interface with a nil concrete pointer, defeating the h.orthrusResolver == nil guard and causing a panic on GetProxyAddr. Two complementary fixes applied: 1. Call site (routes.go): only invoke SetOrthrusResolver when orthrusServer is non-nil, consistent with the existing guard on SetOrthrusServer. 2. Defense-in-depth (docker_handler.go): SetOrthrusResolver now normalizes typed-nil concrete pointers to a proper nil interface via reflect, preventing the trap regardless of call site. Adds a typed-nil regression test that verifies the handler returns 503 rather than panicking when SetOrthrusResolver receives a nil *orthrus.OrthrusServer.
Problem
After switching a
RemoteServerfrom a genericsocatsocket proxy to an Orthrus agent, all Docker operations fail with:Root cause: The Orthrus agent already handles
streamTypeDocker = 0x01— when the server opens a yamux stream with that type byte, the agent bridges it to its local/var/run/docker.sock. However, the server-side half was a stub (proxyPortfield always0,GetProxyAddr()always returns""). Charon's Docker handler had no way to route requests through an agent and fell back totcp://host:portwhich points at nothing.What This PR Does
backend/internal/orthrus/session.goStartDockerProxy()— binds an ephemeral127.0.0.1:NTCP listener when an agent connects; idempotency guard (s.listener != nil) prevents double-allocationrunProxyListener()— accept loop that spawnsproxyConngoroutinesproxyConn()— for each TCP connection: opens a yamux stream, writes type byte0x01, bidirectionally copies via twoio.Copygoroutines +sync.WaitGroupGetProxyAddr()— returns"127.0.0.1:N"once the proxy is live; empty string otherwiseClose()— closes the listener under the mutex; in-flightproxyConngoroutines complete asynchronously as yamux stream close propagatesbackend/internal/orthrus/server.gosess.StartDockerProxy()after each agent WebSocket connects; logs but does not abort on failure (Docker is optional)backend/internal/api/handlers/docker_handler.goorthrusProxyResolverinterface +SetOrthrusResolver()setterListContainers(and other Docker methods) now branch onconnection_type == "orthrus":OrthrusAgentUUID == nil→ 400orthrusResolver == nil(no encryption key) → 503 with explanationtcp://127.0.0.1:Nas Docker hostbackend/internal/api/routes/routes.goorthrusServerdeclaration before the encryption-key block; wires it intodockerHandlerafter construction (nil is safe — handler returns 503)Tests
12 new acceptance criteria, all green:
TestAgentSession_StartDockerProxy_ListensOnLoopback127.0.0.1TestAgentSession_StartDockerProxy_GetProxyAddrTestAgentSession_StartDockerProxy_CalledTwiceTestAgentSession_Close_ClosesListenerTestAgentSession_ProxyConn_WritesStreamTypeByte0x01TestDockerHandler_ListContainers_OrthrusNotConnectedTestDockerHandler_ListContainers_OrthrusNoResolverTestDockerHandler_ListContainers_OrthrusConnectedTestDockerHandler_ListContainers_OrthrusMissingUUID//go:build integration)Coverage: 88.6% lines / 88.5% statements (gate: 87%). Race detector: clean.
Security Notes
127.0.0.1(loopback) — never exposed outside the host:0) — no fixed port to block or exploitmuzzlefilter already restricts allowed Docker API paths — unchangedChecklist