diff --git a/SECURITY.md b/SECURITY.md index ae4153dc1..90be2c5ff 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -756,4 +756,4 @@ We recognize security researchers who help improve Charon: --- -**Last Updated**: 2026-03-24 +**Last Updated**: 2026-05-18 diff --git a/backend/internal/api/handlers/crowdsec_coverage_target_test.go b/backend/internal/api/handlers/crowdsec_coverage_target_test.go index cfed2d5aa..4b93fbdc9 100644 --- a/backend/internal/api/handlers/crowdsec_coverage_target_test.go +++ b/backend/internal/api/handlers/crowdsec_coverage_target_test.go @@ -283,16 +283,16 @@ func TestRegisterBouncerExecutionFailure(t *testing.T) { // TestGetAcquisitionConfigFileError tests file read error func TestGetAcquisitionConfigNotPresent(t *testing.T) { - t.Setenv("CHARON_CROWDSEC_ACQUIS_PATH", filepath.Join(t.TempDir(), "nonexistent.yaml")) - h := newTestCrowdsecHandler(t, OpenTestDB(t), &fakeExec{}, "/bin/false", t.TempDir()) - r := gin.New() - g := r.Group("/api/v1") - h.RegisterRoutes(g) - - w := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/crowdsec/acquisition", http.NoBody) - r.ServeHTTP(w, req) - - // File won't exist in test env, should give 404 - require.Equal(t, http.StatusNotFound, w.Code) + t.Setenv("CHARON_CROWDSEC_ACQUIS_PATH", filepath.Join(t.TempDir(), "nonexistent.yaml")) + h := newTestCrowdsecHandler(t, OpenTestDB(t), &fakeExec{}, "/bin/false", t.TempDir()) + r := gin.New() + g := r.Group("/api/v1") + h.RegisterRoutes(g) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/crowdsec/acquisition", http.NoBody) + r.ServeHTTP(w, req) + + // File won't exist in test env, should give 404 + require.Equal(t, http.StatusNotFound, w.Code) } diff --git a/backend/internal/api/handlers/crowdsec_stop_lapi_test.go b/backend/internal/api/handlers/crowdsec_stop_lapi_test.go index b305b0371..ba0d4fe6e 100644 --- a/backend/internal/api/handlers/crowdsec_stop_lapi_test.go +++ b/backend/internal/api/handlers/crowdsec_stop_lapi_test.go @@ -167,10 +167,10 @@ func TestGetLAPIDecisions_WithMockServer(t *testing.T) { secSvc := createTestSecurityService(t, db) h := &CrowdsecHandler{ - DB: db, - Security: secSvc, - CmdExec: &mockCommandExecutor{}, - DataDir: t.TempDir(), + DB: db, + Security: secSvc, + CmdExec: &mockCommandExecutor{}, + DataDir: t.TempDir(), validateLAPIURL: permissiveLAPIURLValidator, } @@ -210,10 +210,10 @@ func TestGetLAPIDecisions_Unauthorized(t *testing.T) { secSvc := createTestSecurityService(t, db) h := &CrowdsecHandler{ - DB: db, - Security: secSvc, - CmdExec: &mockCommandExecutor{}, - DataDir: t.TempDir(), + DB: db, + Security: secSvc, + CmdExec: &mockCommandExecutor{}, + DataDir: t.TempDir(), validateLAPIURL: permissiveLAPIURLValidator, } @@ -245,10 +245,10 @@ func TestGetLAPIDecisions_NullResponse(t *testing.T) { secSvc := createTestSecurityService(t, db) h := &CrowdsecHandler{ - DB: db, - Security: secSvc, - CmdExec: &mockCommandExecutor{}, - DataDir: t.TempDir(), + DB: db, + Security: secSvc, + CmdExec: &mockCommandExecutor{}, + DataDir: t.TempDir(), validateLAPIURL: permissiveLAPIURLValidator, } @@ -323,10 +323,10 @@ func TestCheckLAPIHealth_WithMockServer(t *testing.T) { secSvc := createTestSecurityService(t, db) h := &CrowdsecHandler{ - DB: db, - Security: secSvc, - CmdExec: &mockCommandExecutor{}, - DataDir: t.TempDir(), + DB: db, + Security: secSvc, + CmdExec: &mockCommandExecutor{}, + DataDir: t.TempDir(), validateLAPIURL: permissiveLAPIURLValidator, } @@ -369,10 +369,10 @@ func TestCheckLAPIHealth_FallbackToDecisions(t *testing.T) { secSvc := createTestSecurityService(t, db) h := &CrowdsecHandler{ - DB: db, - Security: secSvc, - CmdExec: &mockCommandExecutor{}, - DataDir: t.TempDir(), + DB: db, + Security: secSvc, + CmdExec: &mockCommandExecutor{}, + DataDir: t.TempDir(), validateLAPIURL: permissiveLAPIURLValidator, } diff --git a/backend/internal/api/handlers/crowdsec_wave5_test.go b/backend/internal/api/handlers/crowdsec_wave5_test.go index f731d8321..43a1a1cff 100644 --- a/backend/internal/api/handlers/crowdsec_wave5_test.go +++ b/backend/internal/api/handlers/crowdsec_wave5_test.go @@ -35,7 +35,6 @@ func TestCrowdsecWave5_GetLAPIDecisions_Unauthorized(t *testing.T) { })) t.Cleanup(server.Close) - require.NoError(t, db.Create(&models.SecurityConfig{UUID: "default", CrowdSecAPIURL: server.URL}).Error) h := newTestCrowdsecHandler(t, db, &fakeExec{}, "/bin/false", tmpDir) @@ -63,7 +62,6 @@ func TestCrowdsecWave5_GetLAPIDecisions_NonJSONContentTypeFallsBack(t *testing.T })) t.Cleanup(server.Close) - require.NoError(t, db.Create(&models.SecurityConfig{UUID: "default", CrowdSecAPIURL: server.URL}).Error) h := newTestCrowdsecHandler(t, db, &fakeExec{}, "/bin/false", tmpDir) diff --git a/backend/internal/api/handlers/docker_handler.go b/backend/internal/api/handlers/docker_handler.go index 945339b31..6fff588ba 100644 --- a/backend/internal/api/handlers/docker_handler.go +++ b/backend/internal/api/handlers/docker_handler.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net/http" + "reflect" "strings" "github.com/Wikid82/charon/backend/internal/api/middleware" @@ -22,9 +23,14 @@ type remoteServerGetter interface { GetByUUID(uuidStr string) (*models.RemoteServer, error) } +type orthrusProxyResolver interface { + GetProxyAddr(agentUUID string) (string, bool) +} + type DockerHandler struct { dockerService dockerContainerLister remoteServerService remoteServerGetter + orthrusResolver orthrusProxyResolver } func NewDockerHandler(dockerService dockerContainerLister, remoteServerService remoteServerGetter) *DockerHandler { @@ -34,6 +40,17 @@ func NewDockerHandler(dockerService dockerContainerLister, remoteServerService r } } +func (h *DockerHandler) SetOrthrusResolver(r orthrusProxyResolver) { + // Guard against the Go typed-nil trap: a nil *T passed as an interface + // produces a non-nil interface (type descriptor present, data nil), which + // would bypass the h.orthrusResolver == nil guard and panic in GetProxyAddr. + if r == nil || (reflect.ValueOf(r).Kind() == reflect.Ptr && reflect.ValueOf(r).IsNil()) { + h.orthrusResolver = nil + return + } + h.orthrusResolver = r +} + func (h *DockerHandler) RegisterRoutes(r *gin.RouterGroup) { r.GET("/docker/containers", h.ListContainers) } @@ -62,9 +79,25 @@ func (h *DockerHandler) ListContainers(c *gin.Context) { } // Construct Docker host string - // Assuming TCP for now as that's what RemoteServer supports (Host/Port) - // TODO: Support SSH if/when RemoteServer supports it - host = fmt.Sprintf("tcp://%s:%d", server.Host, server.Port) + switch server.ConnectionType { + case models.ConnectionTypeOrthrus: + if h.orthrusResolver == nil { + c.JSON(http.StatusServiceUnavailable, gin.H{"error": "Orthrus subsystem unavailable"}) + return + } + if server.OrthrusAgentUUID == nil || *server.OrthrusAgentUUID == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "Remote server has no Orthrus agent UUID configured"}) + return + } + proxyAddr, ok := h.orthrusResolver.GetProxyAddr(*server.OrthrusAgentUUID) + if !ok { + c.JSON(http.StatusBadGateway, gin.H{"error": "Orthrus agent is not currently connected"}) + return + } + host = "tcp://" + proxyAddr + default: + host = fmt.Sprintf("tcp://%s:%d", server.Host, server.Port) + } } containers, err := h.dockerService.ListContainers(c.Request.Context(), host) diff --git a/backend/internal/api/handlers/docker_handler_test.go b/backend/internal/api/handlers/docker_handler_test.go index 73cc811d2..7cc9c0258 100644 --- a/backend/internal/api/handlers/docker_handler_test.go +++ b/backend/internal/api/handlers/docker_handler_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/Wikid82/charon/backend/internal/models" + "github.com/Wikid82/charon/backend/internal/orthrus" "github.com/Wikid82/charon/backend/internal/services" "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" @@ -392,3 +393,169 @@ func TestDockerHandler_ListContainers_503DetailsWithGroupGuidance(t *testing.T) assert.Contains(t, w.Body.String(), "--group-add 988") assert.Contains(t, w.Body.String(), "group_add") } + +type fakeOrthrusResolver struct { + addr string + ok bool +} + +func (f *fakeOrthrusResolver) GetProxyAddr(_ string) (string, bool) { + return f.addr, f.ok +} + +func TestDockerHandler_ListContainers_OrthrusAgentConnected(t *testing.T) { + router := gin.New() + agentUUID := "agent-uuid-123" + dockerSvc := &fakeDockerService{ret: []services.DockerContainer{}} + remoteSvc := &fakeRemoteServerService{server: &models.RemoteServer{ + ConnectionType: models.ConnectionTypeOrthrus, + OrthrusAgentUUID: &agentUUID, + }} + h := NewDockerHandler(dockerSvc, remoteSvc) + h.SetOrthrusResolver(&fakeOrthrusResolver{addr: "127.0.0.1:54321", ok: true}) + + api := router.Group("/api/v1") + h.RegisterRoutes(api) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/docker/containers?server_id=srv-1", http.NoBody) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + require.True(t, dockerSvc.called) + assert.Equal(t, "tcp://127.0.0.1:54321", dockerSvc.host) + assert.Equal(t, http.StatusOK, w.Code) +} + +func TestDockerHandler_ListContainers_OrthrusAgentOffline(t *testing.T) { + router := gin.New() + agentUUID := "agent-offline-uuid" + dockerSvc := &fakeDockerService{} + remoteSvc := &fakeRemoteServerService{server: &models.RemoteServer{ + ConnectionType: models.ConnectionTypeOrthrus, + OrthrusAgentUUID: &agentUUID, + }} + h := NewDockerHandler(dockerSvc, remoteSvc) + h.SetOrthrusResolver(&fakeOrthrusResolver{ok: false}) + + api := router.Group("/api/v1") + h.RegisterRoutes(api) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/docker/containers?server_id=srv-1", http.NoBody) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusBadGateway, w.Code) + assert.Contains(t, w.Body.String(), "Orthrus agent is not currently connected") + assert.False(t, dockerSvc.called) +} + +func TestDockerHandler_ListContainers_OrthrusSubsystemUnavailable(t *testing.T) { + router := gin.New() + agentUUID := "agent-uuid-svc" + dockerSvc := &fakeDockerService{} + remoteSvc := &fakeRemoteServerService{server: &models.RemoteServer{ + ConnectionType: models.ConnectionTypeOrthrus, + OrthrusAgentUUID: &agentUUID, + }} + h := NewDockerHandler(dockerSvc, remoteSvc) + // orthrusResolver intentionally not set (nil) + + api := router.Group("/api/v1") + h.RegisterRoutes(api) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/docker/containers?server_id=srv-1", http.NoBody) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusServiceUnavailable, w.Code) + assert.Contains(t, w.Body.String(), "Orthrus subsystem unavailable") + assert.False(t, dockerSvc.called) +} + +func TestDockerHandler_ListContainers_OrthrusMissingAgentUUID(t *testing.T) { + router := gin.New() + dockerSvc := &fakeDockerService{} + remoteSvc := &fakeRemoteServerService{server: &models.RemoteServer{ + ConnectionType: models.ConnectionTypeOrthrus, + OrthrusAgentUUID: nil, + }} + h := NewDockerHandler(dockerSvc, remoteSvc) + h.SetOrthrusResolver(&fakeOrthrusResolver{ok: true, addr: "127.0.0.1:1234"}) + + api := router.Group("/api/v1") + h.RegisterRoutes(api) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/docker/containers?server_id=srv-1", http.NoBody) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Contains(t, w.Body.String(), "no Orthrus agent UUID configured") + assert.False(t, dockerSvc.called) +} + +func TestDockerHandler_SetOrthrusResolver_Nil(t *testing.T) { + h := NewDockerHandler(&fakeDockerService{}, &fakeRemoteServerService{}) + h.SetOrthrusResolver(nil) + assert.Nil(t, h.orthrusResolver) +} + +// TestDockerHandler_ListContainers_OrthrusEmptyAgentUUID verifies that a +// non-nil pointer to an empty string for OrthrusAgentUUID is rejected with 400. +// This covers the right-hand side of the || condition in the UUID guard. +func TestDockerHandler_ListContainers_OrthrusEmptyAgentUUID(t *testing.T) { + gin.SetMode(gin.TestMode) + router := gin.New() + emptyUUID := "" + dockerSvc := &fakeDockerService{} + remoteSvc := &fakeRemoteServerService{server: &models.RemoteServer{ + ConnectionType: models.ConnectionTypeOrthrus, + OrthrusAgentUUID: &emptyUUID, + }} + h := NewDockerHandler(dockerSvc, remoteSvc) + h.SetOrthrusResolver(&fakeOrthrusResolver{ok: true, addr: "127.0.0.1:1234"}) + + api := router.Group("/api/v1") + h.RegisterRoutes(api) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/docker/containers?server_id=srv-1", http.NoBody) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Contains(t, w.Body.String(), "no Orthrus agent UUID configured") + assert.False(t, dockerSvc.called) +} + +// TestDockerHandler_SetOrthrusResolver_TypedNil verifies the Go typed-nil trap: +// passing a nil *orthrus.OrthrusServer to SetOrthrusResolver would normally box +// into a non-nil interface (type descriptor present, data pointer nil), which +// defeats the h.orthrusResolver == nil guard and panics inside GetProxyAddr. +// SetOrthrusResolver must normalise such values to a truly-nil interface so the +// 503 path is taken instead. +func TestDockerHandler_SetOrthrusResolver_TypedNil(t *testing.T) { + gin.SetMode(gin.TestMode) + + var typedNil *orthrus.OrthrusServer + uuid := "test-agent-uuid" + remoteSvc := &fakeRemoteServerService{server: &models.RemoteServer{ + ConnectionType: models.ConnectionTypeOrthrus, + OrthrusAgentUUID: &uuid, + }} + h := NewDockerHandler(&fakeDockerService{}, remoteSvc) + h.SetOrthrusResolver(typedNil) + + // Resolver must be a truly-nil interface, not a non-nil typed-nil. + assert.Nil(t, h.orthrusResolver) + + router := gin.New() + api := router.Group("/api/v1") + h.RegisterRoutes(api) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/docker/containers?server_id=srv-1", http.NoBody) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) // must NOT panic + + assert.Equal(t, http.StatusServiceUnavailable, w.Code) + assert.Contains(t, w.Body.String(), "Orthrus subsystem unavailable") +} diff --git a/backend/internal/api/handlers/security_notifications_single_source_test.go b/backend/internal/api/handlers/security_notifications_single_source_test.go index 405161eb1..b763d4cf4 100644 --- a/backend/internal/api/handlers/security_notifications_single_source_test.go +++ b/backend/internal/api/handlers/security_notifications_single_source_test.go @@ -125,7 +125,6 @@ func TestR6_LegacySecuritySettingsWrite410Gone(t *testing.T) { service := services.NewEnhancedSecurityNotificationService(db) handler := NewSecurityNotificationHandler(service) - // Test canonical endpoint: PUT /api/v1/notifications/settings/security t.Run("CanonicalEndpoint", func(t *testing.T) { reqBody := map[string]interface{}{ @@ -203,7 +202,6 @@ func TestR6_LegacyWrite410GoneNoMutation(t *testing.T) { service := services.NewEnhancedSecurityNotificationService(db) handler := NewSecurityNotificationHandler(service) - // Attempt PUT to canonical endpoint reqBody := map[string]interface{}{ "security_waf_enabled": true, @@ -237,7 +235,6 @@ func TestProviderCRUD_SecurityEventsIncludeCrowdSec(t *testing.T) { service := services.NewNotificationService(db, nil) handler := NewNotificationProviderHandler(service) - // Test CREATE t.Run("CreatePersistsCrowdSec", func(t *testing.T) { reqBody := notificationProviderUpsertRequest{ diff --git a/backend/internal/api/handlers/system_permissions_handler_test.go b/backend/internal/api/handlers/system_permissions_handler_test.go index 843a6dd1e..4764c848c 100644 --- a/backend/internal/api/handlers/system_permissions_handler_test.go +++ b/backend/internal/api/handlers/system_permissions_handler_test.go @@ -103,7 +103,6 @@ func TestSystemPermissionsHandler_RepairPermissions_NonRoot(t *testing.T) { t.Skip("test requires non-root execution") } - cfg := config.Config{SingleContainer: true} h := NewSystemPermissionsHandler(cfg, nil, stubPermissionChecker{}) @@ -232,7 +231,6 @@ func TestSystemPermissionsHandler_RepairPermissions_InvalidJSON(t *testing.T) { t.Skip("test requires root execution") } - root := t.TempDir() dataDir := filepath.Join(root, "data") require.NoError(t, os.MkdirAll(dataDir, 0o750)) @@ -264,7 +262,6 @@ func TestSystemPermissionsHandler_RepairPermissions_Success(t *testing.T) { t.Skip("test requires root execution") } - root := t.TempDir() dataDir := filepath.Join(root, "data") require.NoError(t, os.MkdirAll(dataDir, 0o750)) @@ -526,7 +523,6 @@ func TestSystemPermissionsHandler_RepairPermissions_InvalidRequestBody_Root(t *t t.Skip("test requires root execution") } - tmp := t.TempDir() dataDir := filepath.Join(tmp, "data") require.NoError(t, os.MkdirAll(dataDir, 0o750)) diff --git a/backend/internal/api/handlers/system_permissions_wave6_test.go b/backend/internal/api/handlers/system_permissions_wave6_test.go index 09d34c939..ae6a2366a 100644 --- a/backend/internal/api/handlers/system_permissions_wave6_test.go +++ b/backend/internal/api/handlers/system_permissions_wave6_test.go @@ -28,7 +28,6 @@ func TestSystemPermissionsWave6_RepairPermissions_NonRootBranchViaSeteuid(t *tes require.NoError(t, restoreErr) }() - root := t.TempDir() dataDir := filepath.Join(root, "data") require.NoError(t, os.MkdirAll(dataDir, 0o750)) diff --git a/backend/internal/api/routes/routes.go b/backend/internal/api/routes/routes.go index 1ed53775b..9d87fef04 100644 --- a/backend/internal/api/routes/routes.go +++ b/backend/internal/api/routes/routes.go @@ -389,6 +389,7 @@ func RegisterWithDeps(ctx context.Context, router *gin.Engine, db *gorm.DB, cfg management.DELETE("/domains/:id", domainHandler.Delete) // DNS Providers - only available if encryption key is configured + var orthrusServer *orthrus.OrthrusServer if cfg.EncryptionKey != "" { encryptionService, err := crypto.NewEncryptionService(cfg.EncryptionKey) if err != nil { @@ -467,7 +468,6 @@ func RegisterWithDeps(ctx context.Context, router *gin.Engine, db *gorm.DB, cfg } orthrusCA, caErr := orthrus.NewInternalCA(dataRoot) - var orthrusServer *orthrus.OrthrusServer if caErr == nil { var serverErr error orthrusServer, serverErr = orthrus.NewOrthrusServer(db, orthrusCA) @@ -503,6 +503,9 @@ func RegisterWithDeps(ctx context.Context, router *gin.Engine, db *gorm.DB, cfg // The service will return proper error messages when Docker is not accessible dockerService := services.NewDockerService() dockerHandler := handlers.NewDockerHandler(dockerService, remoteServerService) + if orthrusServer != nil { + dockerHandler.SetOrthrusResolver(orthrusServer) + } dockerHandler.RegisterRoutes(management) // Uptime Service — reuse the single uptimeService instance (defined above) diff --git a/backend/internal/config/config.go b/backend/internal/config/config.go index fe09bce32..f498119f3 100644 --- a/backend/internal/config/config.go +++ b/backend/internal/config/config.go @@ -15,27 +15,27 @@ import ( // Config captures runtime configuration sourced from environment variables. type Config struct { - Environment string - HTTPPort string - DatabasePath string - ConfigRoot string - FrontendDir string - CaddyAdminAPI string - CaddyConfigDir string - CaddyBinary string - ImportCaddyfile string - ImportDir string - JWTSecret string - EncryptionKey string - ACMEStaging bool - SingleContainer bool - PluginsDir string - CaddyLogDir string - CrowdSecLogDir string - Debug bool + Environment string + HTTPPort string + DatabasePath string + ConfigRoot string + FrontendDir string + CaddyAdminAPI string + CaddyConfigDir string + CaddyBinary string + ImportCaddyfile string + ImportDir string + JWTSecret string + EncryptionKey string + ACMEStaging bool + SingleContainer bool + PluginsDir string + CaddyLogDir string + CrowdSecLogDir string + Debug bool CertExpiryWarningDays int - Security SecurityConfig - Emergency EmergencyConfig + Security SecurityConfig + Emergency EmergencyConfig } // SecurityConfig holds configuration for optional security services. diff --git a/backend/internal/orthrus/proxy_integration_test.go b/backend/internal/orthrus/proxy_integration_test.go new file mode 100644 index 000000000..efd27983b --- /dev/null +++ b/backend/internal/orthrus/proxy_integration_test.go @@ -0,0 +1,9 @@ +//go:build integration + +package orthrus + +import "testing" + +func TestDockerProxy_Integration(t *testing.T) { + t.Skip("requires running Orthrus agent with /var/run/docker.sock") +} diff --git a/backend/internal/services/certificate_validator_extra_coverage_test.go b/backend/internal/services/certificate_validator_extra_coverage_test.go index 5d20f49c6..f2a8f3941 100644 --- a/backend/internal/services/certificate_validator_extra_coverage_test.go +++ b/backend/internal/services/certificate_validator_extra_coverage_test.go @@ -252,5 +252,3 @@ func TestDetectFormat_PEM(t *testing.T) { format := DetectFormat(certPEM) assert.Equal(t, FormatPEM, format) } - - diff --git a/docs/plans/current_spec.md b/docs/plans/current_spec.md index 9816b8cbc..a45b0347e 100644 --- a/docs/plans/current_spec.md +++ b/docs/plans/current_spec.md @@ -1,13 +1,9 @@ -# CI Fix — ESLint + Backend Test Failures +# Orthrus Agent Docker Proxy Listener — Feature Spec (PR 5) -**Branch**: `fix/ci-eslint-backend-test` -**PR**: Targets `development` -**Date**: 2026-05-29 - ---- - -> **Archived**: The previous spec (CI Fix — Vitest `invites a new user` Failure) has been -> superseded. This document covers the active CI failures. +**Branch**: `feature/hecate` +**PR**: #5 (Orthrus Docker Proxy — server-side listener) +**Date**: 2026-05-18 +**Status**: Ready for implementation --- @@ -15,708 +11,707 @@ ### Overview -Multiple CI jobs in `quality-checks.yml` are failing. The failures split across two domains: +Orthrus agents connect to Charon over a persistent WebSocket multiplexed with yamux. The agent binary (`agent/leash/leash.go`) already handles an incoming yamux stream whose first byte is `0x01` (`streamTypeDocker`) by connecting to its local `/var/run/docker.sock` and bidirectionally copying. The server-side half of this tunnel was deferred with the comment "PR 5". -1. **Frontend ESLint** — Blocking job; 7 distinct lint violations across 5 source files and 1 - test file. -2. **Backend test suite** — Blocking job (via `scripts/go-test-coverage.sh`); - `TestSecurityHandler_UpsertRuleSet_XSSInContent` fails when the full handler package test suite - runs in parallel. +This spec covers the complete implementation: -Backend **lint** violations exist but are configured with `continue-on-error: true` and are -therefore non-blocking. They are explicitly excluded from this plan. +1. **Server-side proxy listener** — when an agent session is registered, allocate an ephemeral `127.0.0.1:0` TCP listener; for each accepted connection, open a yamux stream to the agent, write `0x01`, then bidirectionally copy. +2. **`DockerHandler` integration** — when a `RemoteServer` has `connection_type == "orthrus"`, resolve the agent's proxy address and use `tcp://127.0.0.1:` as the Docker host instead of the server's `Host:Port` fields. ### Objectives -- Restore all CI jobs to green in a single PR composed of 6 ordered, reviewable commits. -- No feature changes; this is a pure fix/refactor to make the test suite and linter pass. +1. `GetProxyAddr()` returns a non-empty address for every live agent session. +2. Docker container listing via `GET /api/v1/docker/containers?server_id=` works for Orthrus-backed remote servers. +3. Clear 502/503 errors when the agent is offline or the orthrus subsystem is unavailable. +4. All changes are covered by unit tests; an integration test stub is provided. --- ## 2. Research Findings -### 2.1 CI Workflow +### 2.1 `backend/internal/orthrus/session.go` -**File**: `.github/workflows/quality-checks.yml` +**`AgentSession` struct** (relevant fields): -| Job | Script / Command | Blocking? | -|-----|-----------------|-----------| -| Backend tests + coverage | `scripts/go-test-coverage.sh` | **Yes** | -| Backend lint | `golangci/golangci-lint-action@v9.2.0` with `continue-on-error: true` | No | -| Frontend ESLint | `npm run lint` | **Yes** | -| Frontend type-check | `npm run type-check` | Yes | -| Frontend unit tests | `scripts/frontend-test-coverage.sh` | Yes | +```go +type AgentSession struct { + agentUUID string + agentName string + conn *websocket.Conn + session *yamux.Session + cancel context.CancelFunc + proxyPort int // 0 means no proxy listener yet (allocated in PR 5) + mu sync.Mutex +} +``` -`scripts/go-test-coverage.sh` captures `GO_TEST_STATUS` from the test run, applies the coverage -gate, and at the end bubbles up any non-zero test exit code (script lines 290-295). A single -failing test therefore causes the job to exit non-zero. +**`GetProxyAddr()`** already returns the correct format, updated to use `s.listener != nil` as the sentinel for consistency with the idempotency guard: -### 2.2 Frontend ESLint — Root Causes +```go +func (s *AgentSession) GetProxyAddr() string { + s.mu.Lock() + defer s.mu.Unlock() + if s.listener == nil { + return "" + } + return fmt.Sprintf("127.0.0.1:%d", s.proxyPort) +} +``` + +An existing coverage test (`session_coverage_test.go`) manually sets `sess.proxyPort = 8080` and asserts `GetProxyAddr()` returns `"127.0.0.1:8080"` — the integer field remains the source of truth for the port value; `listener` is the guard that determines whether a proxy is active. + +**`Close()`** cancels the context and closes the yamux session (which also closes the underlying WebSocket). The listener allocated in PR 5 must be closed here. + +**`IsAlive()`** returns `!s.session.IsClosed()`. -**ESLint plugins in use** (`frontend/eslint.config.js`): -`jsx-a11y`, `security`, `react-refresh`, `@vitest/eslint-plugin` (aliased as `vitest`) +### 2.2 `backend/internal/orthrus/server.go` -#### Failure 1 — Duplicate devDependencies +**`OrthrusServer`** stores sessions in a `sync.Map` keyed by `agentUUID → *AgentSession`. -`frontend/package.json` contains three devDependency keys that each appear twice: +**`HandleWebSocket()`** — the connection lifecycle: +1. Authenticates the bearer token (bcrypt compare against all agents). +2. Calls `NewAgentSession(uuid, name, conn)` to create the yamux server session. +3. Stores in `sessions`. +4. Updates the agent's `status = online` in the DB. +5. Launches `watchHeartbeat` goroutine. -| Duplicate key | Affected lines | -|---|---| -| `@typescript-eslint/eslint-plugin` | first occurrence + second set ~lines 74-76 | -| `@typescript-eslint/parser` | same | -| `@typescript-eslint/utils` | same | +`StartDockerProxy()` (to be added) must be called between steps 2 and 3. -npm treats a duplicate key as a parse-time error that prevents consistent lock-file resolution, -causing the ESLint run to fail with a dependency error. +**`watchHeartbeat()`** polls `sess.IsAlive()` on a ticker. When false, it calls `markOffline` and `s.sessions.Delete(agentUUID)` — but does **not** call `sess.Close()`. This means the listener goroutine would outlive the session. The fix: call `sess.Close()` in `watchHeartbeat` when `!sess.IsAlive()`. -#### Failure 2 — Unsafe regex (`security/detect-unsafe-regex`) +**`GetProxyAddr(agentUUID string) (string, bool)`** and **`GetSession(agentUUID string) (*AgentSession, bool)`** are already implemented as stubs and will work correctly once `proxyPort` is set. -**File**: `frontend/src/components/CredentialManager.tsx` +**`Stop()`** already calls `sess.Close()` for all sessions — listener cleanup will be covered once `Close()` is updated. -`validateZoneFilter` uses a regex with nested quantifiers. The rule flags it as a potential ReDoS -vector. The regex is intentional; the risk is acceptable for this context. +### 2.3 `backend/internal/api/handlers/docker_handler.go` -**Fix**: Add an inline `// eslint-disable-next-line security/detect-unsafe-regex` with a -justification comment immediately before the regex literal. +**Current struct**: -#### Failure 3 — Non-component exports (`react-refresh/only-export-components`) +```go +type DockerHandler struct { + dockerService dockerContainerLister + remoteServerService remoteServerGetter +} +``` -**File**: `frontend/src/components/CertificateList.tsx` — lines 20 and 24 +**Current `ListContainers` flow for `server_id`**: -```tsx -export function isInUse(cert: Certificate): boolean { ... } // line 20 -export function isDeletable(cert: Certificate): boolean { ... } // line 24 +```go +server, err := h.remoteServerService.GetByUUID(serverID) +// ... +host = fmt.Sprintf("tcp://%s:%d", server.Host, server.Port) ``` -`react-refresh` requires component files export **only React components**. Exporting plain utility -functions breaks HMR and triggers this rule. +No awareness of `ConnectionType` or `OrthrusAgentUUID`. No reference to `OrthrusServer`. -**Known import sites** (must be updated after move): -- `frontend/src/components/__tests__/CertificateList.test.tsx` line 8 +### 2.4 `backend/internal/api/routes/routes.go` -**Fix**: Move both functions to a new `frontend/src/utils/certificateUtils.ts`; update all import -sites; keep internal calls in `CertificateList.tsx` via the new import. +**Critical wiring gap**: `orthrusServer` is created inside `if strings.TrimSpace(os.Getenv("CHARON_ENCRYPTION_KEY")) != "" { ... }`. The `dockerHandler` is created **after** this block closes: -#### Failure 4 — Label on non-labelable elements (`jsx-a11y/label-has-associated-control`) +```go +if encryptionKey != "" { + // ... + orthrusServer, _ = orthrus.NewOrthrusServer(db, orthrusCA) + // orthrusServer registered with api and caddyManager here +} +// dockerHandler created here — orthrusServer is OUT OF SCOPE +dockerService := services.NewDockerService() +dockerHandler := handlers.NewDockerHandler(dockerService, remoteServerService) +dockerHandler.RegisterRoutes(management) +``` -5 violations across 3 files: +Fix: hoist `var orthrusServer *orthrus.OrthrusServer` declaration above the `if` block, then call `dockerHandler.SetOrthrusResolver(orthrusServer)` (may be nil) after the handler is created. -| File | Line | Element | Problem | -|---|---|---|---| -| `frontend/src/components/CSPBuilder.tsx` | ~326 | `