diff --git a/.docker/docker-entrypoint.sh b/.docker/docker-entrypoint.sh index 599f3af2..9bbad5d0 100755 --- a/.docker/docker-entrypoint.sh +++ b/.docker/docker-entrypoint.sh @@ -30,6 +30,27 @@ mkdir -p /app/data/caddy 2>/dev/null || true mkdir -p /app/data/crowdsec 2>/dev/null || true mkdir -p /app/data/geoip 2>/dev/null || true +# ============================================================================ +# Docker Socket Permission Handling +# ============================================================================ +# The Docker integration feature requires access to the Docker socket. +# When running as non-root user (charon), we need to ensure the user is in +# the same group as the mounted socket for permission access. + +if [ -S "/var/run/docker.sock" ]; then + DOCKER_SOCK_GID=$(stat -c '%g' /var/run/docker.sock 2>/dev/null || echo "") + if [ -n "$DOCKER_SOCK_GID" ] && [ "$DOCKER_SOCK_GID" != "0" ]; then + # Check if a group with this GID exists + if ! getent group "$DOCKER_SOCK_GID" >/dev/null 2>&1; then + echo "Docker socket detected (gid=$DOCKER_SOCK_GID). Note: Container integration requires socket access." + echo " To enable Docker container discovery:" + echo " 1. Run container with --user root:root, OR" + echo " 2. Add host docker group: docker run --group-add $DOCKER_SOCK_GID ..., OR" + echo " 3. Change socket permissions: chmod 666 /var/run/docker.sock (not recommended)" + fi + fi +fi + # ============================================================================ # CrowdSec Initialization # ============================================================================ diff --git a/.vscode/tasks.json b/.vscode/tasks.json index 229618bd..f213a842 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -4,7 +4,7 @@ { "label": "Build & Run: Local Docker Image", "type": "shell", - "command": "docker build -t charon:local . && docker compose -f docker-compose.override.yml up -d && echo 'Charon running at http://localhost:8080'", + "command": "docker build -t charon:local . && docker compose -f docker-compose.test.yml up -d && echo 'Charon running at http://localhost:8080'", "group": "build", "problemMatcher": [], "presentation": { @@ -15,7 +15,7 @@ { "label": "Build & Run: Local Docker Image No-Cache", "type": "shell", - "command": "docker build --no-cache -t charon:local . && docker compose -f docker-compose.override.yml up -d && echo 'Charon running at http://localhost:8080'", + "command": "docker build --no-cache -t charon:local . && docker compose -f docker-compose.test.yml up -d && echo 'Charon running at http://localhost:8080'", "group": "build", "problemMatcher": [], "presentation": { diff --git a/backend/internal/api/handlers/docker_handler.go b/backend/internal/api/handlers/docker_handler.go index 1f4540c6..6a105122 100644 --- a/backend/internal/api/handlers/docker_handler.go +++ b/backend/internal/api/handlers/docker_handler.go @@ -1,19 +1,33 @@ package handlers import ( + "context" + "errors" "fmt" "net/http" + "strings" + "github.com/Wikid82/charon/backend/internal/api/middleware" + "github.com/Wikid82/charon/backend/internal/models" "github.com/Wikid82/charon/backend/internal/services" + "github.com/Wikid82/charon/backend/internal/util" "github.com/gin-gonic/gin" ) -type DockerHandler struct { - dockerService *services.DockerService - remoteServerService *services.RemoteServerService +type dockerContainerLister interface { + ListContainers(ctx context.Context, host string) ([]services.DockerContainer, error) } -func NewDockerHandler(dockerService *services.DockerService, remoteServerService *services.RemoteServerService) *DockerHandler { +type remoteServerGetter interface { + GetByUUID(uuidStr string) (*models.RemoteServer, error) +} + +type DockerHandler struct { + dockerService dockerContainerLister + remoteServerService remoteServerGetter +} + +func NewDockerHandler(dockerService dockerContainerLister, remoteServerService remoteServerGetter) *DockerHandler { return &DockerHandler{ dockerService: dockerService, remoteServerService: remoteServerService, @@ -25,13 +39,24 @@ func (h *DockerHandler) RegisterRoutes(r *gin.RouterGroup) { } func (h *DockerHandler) ListContainers(c *gin.Context) { - host := c.Query("host") - serverID := c.Query("server_id") + log := middleware.GetRequestLogger(c) + + host := strings.TrimSpace(c.Query("host")) + serverID := strings.TrimSpace(c.Query("server_id")) + + // SSRF hardening: do not accept arbitrary host values from the client. + // Only allow explicit local selection ("local") or empty (default local). + if host != "" && host != "local" { + log.WithFields(map[string]any{"host": util.SanitizeForLog(host)}).Warn("rejected docker host query param") + c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid docker host selector"}) + return + } // If server_id is provided, look up the remote server if serverID != "" { server, err := h.remoteServerService.GetByUUID(serverID) if err != nil { + log.WithFields(map[string]any{"server_id": serverID}).Warn("remote server not found") c.JSON(http.StatusNotFound, gin.H{"error": "Remote server not found"}) return } @@ -44,7 +69,15 @@ func (h *DockerHandler) ListContainers(c *gin.Context) { containers, err := h.dockerService.ListContainers(c.Request.Context(), host) if err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to list containers: " + err.Error()}) + var unavailableErr *services.DockerUnavailableError + if errors.As(err, &unavailableErr) { + log.WithFields(map[string]any{"server_id": serverID}).WithError(err).Warn("docker unavailable") + c.JSON(http.StatusServiceUnavailable, gin.H{"error": "Docker daemon unavailable"}) + return + } + + log.WithFields(map[string]any{"server_id": serverID}).WithError(err).Error("failed to list containers") + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to list containers"}) return } diff --git a/backend/internal/api/handlers/docker_handler_test.go b/backend/internal/api/handlers/docker_handler_test.go index 0ac6c1cd..5fec22bc 100644 --- a/backend/internal/api/handlers/docker_handler_test.go +++ b/backend/internal/api/handlers/docker_handler_test.go @@ -1,6 +1,8 @@ package handlers import ( + "context" + "errors" "net/http" "net/http/httptest" "testing" @@ -8,164 +10,110 @@ import ( "github.com/Wikid82/charon/backend/internal/models" "github.com/Wikid82/charon/backend/internal/services" "github.com/gin-gonic/gin" - "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "gorm.io/driver/sqlite" - "gorm.io/gorm" ) -func setupDockerTestRouter(t *testing.T) (*gin.Engine, *gorm.DB, *services.RemoteServerService) { - dsn := "file:" + t.Name() + "?mode=memory&cache=shared" - db, err := gorm.Open(sqlite.Open(dsn), &gorm.Config{}) - require.NoError(t, err) - require.NoError(t, db.AutoMigrate(&models.RemoteServer{})) +type fakeDockerService struct { + called bool + host string - rsService := services.NewRemoteServerService(db) + ret []services.DockerContainer + err error +} +func (f *fakeDockerService) ListContainers(_ context.Context, host string) ([]services.DockerContainer, error) { + f.called = true + f.host = host + return f.ret, f.err +} + +type fakeRemoteServerService struct { + gotUUID string + + server *models.RemoteServer + err error +} + +func (f *fakeRemoteServerService) GetByUUID(uuidStr string) (*models.RemoteServer, error) { + f.gotUUID = uuidStr + return f.server, f.err +} + +func TestDockerHandler_ListContainers_InvalidHostRejected(t *testing.T) { gin.SetMode(gin.TestMode) - r := gin.New() + router := gin.New() - return r, db, rsService + dockerSvc := &fakeDockerService{} + remoteSvc := &fakeRemoteServerService{} + h := NewDockerHandler(dockerSvc, remoteSvc) + + api := router.Group("/api/v1") + h.RegisterRoutes(api) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/docker/containers?host=tcp://127.0.0.1:2375", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.False(t, dockerSvc.called, "docker service should not be called for invalid host") } -func TestDockerHandler_ListContainers(t *testing.T) { - // We can't easily mock the DockerService without an interface, - // and the DockerService depends on the real Docker client. - // So we'll just test that the handler is wired up correctly, - // even if it returns an error because Docker isn't running in the test env. +func TestDockerHandler_ListContainers_DockerUnavailableMappedTo503(t *testing.T) { + gin.SetMode(gin.TestMode) + router := gin.New() - svc, _ := services.NewDockerService() - // svc might be nil if docker is not available, but NewDockerHandler handles nil? - // Actually NewDockerHandler just stores it. - // If svc is nil, ListContainers will panic. - // So we only run this if svc is not nil. + dockerSvc := &fakeDockerService{err: services.NewDockerUnavailableError(errors.New("no docker socket"))} + remoteSvc := &fakeRemoteServerService{} + h := NewDockerHandler(dockerSvc, remoteSvc) - if svc == nil { - t.Skip("Docker not available") - } + api := router.Group("/api/v1") + h.RegisterRoutes(api) - r, _, rsService := setupDockerTestRouter(t) - - h := NewDockerHandler(svc, rsService) - h.RegisterRoutes(r.Group("/")) - - req, _ := http.NewRequest("GET", "/docker/containers", http.NoBody) + req := httptest.NewRequest(http.MethodGet, "/api/v1/docker/containers?host=local", nil) w := httptest.NewRecorder() - r.ServeHTTP(w, req) + router.ServeHTTP(w, req) - // It might return 200 or 500 depending on if ListContainers succeeds - assert.Contains(t, []int{http.StatusOK, http.StatusInternalServerError}, w.Code) + assert.Equal(t, http.StatusServiceUnavailable, w.Code) + assert.Contains(t, w.Body.String(), "Docker daemon unavailable") } -func TestDockerHandler_ListContainers_NonExistentServerID(t *testing.T) { - svc, _ := services.NewDockerService() - if svc == nil { - t.Skip("Docker not available") - } +func TestDockerHandler_ListContainers_ServerIDResolvesToTCPHost(t *testing.T) { + gin.SetMode(gin.TestMode) + router := gin.New() - r, _, rsService := setupDockerTestRouter(t) + dockerSvc := &fakeDockerService{ret: []services.DockerContainer{}} + remoteSvc := &fakeRemoteServerService{server: &models.RemoteServer{Host: "example.internal", Port: 2375}} + h := NewDockerHandler(dockerSvc, remoteSvc) - h := NewDockerHandler(svc, rsService) - h.RegisterRoutes(r.Group("/")) + api := router.Group("/api/v1") + h.RegisterRoutes(api) - // Request with non-existent server_id - req, _ := http.NewRequest("GET", "/docker/containers?server_id=non-existent-uuid", http.NoBody) + req := httptest.NewRequest(http.MethodGet, "/api/v1/docker/containers?server_id=abc-123", nil) w := httptest.NewRecorder() - r.ServeHTTP(w, req) + router.ServeHTTP(w, req) + + require.True(t, dockerSvc.called) + assert.Equal(t, "abc-123", remoteSvc.gotUUID) + assert.Equal(t, "tcp://example.internal:2375", dockerSvc.host) + assert.Equal(t, http.StatusOK, w.Code) +} + +func TestDockerHandler_ListContainers_ServerIDNotFoundReturns404(t *testing.T) { + gin.SetMode(gin.TestMode) + router := gin.New() + + dockerSvc := &fakeDockerService{} + remoteSvc := &fakeRemoteServerService{err: errors.New("not found")} + h := NewDockerHandler(dockerSvc, remoteSvc) + + api := router.Group("/api/v1") + h.RegisterRoutes(api) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/docker/containers?server_id=missing", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) assert.Equal(t, http.StatusNotFound, w.Code) - assert.Contains(t, w.Body.String(), "Remote server not found") -} - -func TestDockerHandler_ListContainers_WithServerID(t *testing.T) { - svc, _ := services.NewDockerService() - if svc == nil { - t.Skip("Docker not available") - } - - r, db, rsService := setupDockerTestRouter(t) - - // Create a remote server - server := models.RemoteServer{ - UUID: uuid.New().String(), - Name: "Test Docker Server", - Host: "docker.example.com", - Port: 2375, - Scheme: "", - Enabled: true, - } - require.NoError(t, db.Create(&server).Error) - - h := NewDockerHandler(svc, rsService) - h.RegisterRoutes(r.Group("/")) - - // Request with valid server_id (will fail to connect, but shouldn't error on lookup) - req, _ := http.NewRequest("GET", "/docker/containers?server_id="+server.UUID, http.NoBody) - w := httptest.NewRecorder() - r.ServeHTTP(w, req) - - // Should attempt to connect and likely fail with 500 (not 404) - assert.Contains(t, []int{http.StatusOK, http.StatusInternalServerError}, w.Code) - if w.Code == http.StatusInternalServerError { - assert.Contains(t, w.Body.String(), "Failed to list containers") - } -} - -func TestDockerHandler_ListContainers_WithHostQuery(t *testing.T) { - svc, _ := services.NewDockerService() - if svc == nil { - t.Skip("Docker not available") - } - - r, _, rsService := setupDockerTestRouter(t) - - h := NewDockerHandler(svc, rsService) - h.RegisterRoutes(r.Group("/")) - - // Request with custom host parameter - req, _ := http.NewRequest("GET", "/docker/containers?host=tcp://invalid-host:2375", http.NoBody) - w := httptest.NewRecorder() - r.ServeHTTP(w, req) - - // Should attempt to connect and fail with 500 - assert.Equal(t, http.StatusInternalServerError, w.Code) - assert.Contains(t, w.Body.String(), "Failed to list containers") -} - -func TestDockerHandler_RegisterRoutes(t *testing.T) { - svc, _ := services.NewDockerService() - if svc == nil { - t.Skip("Docker not available") - } - - r, _, rsService := setupDockerTestRouter(t) - - h := NewDockerHandler(svc, rsService) - h.RegisterRoutes(r.Group("/")) - - // Verify route is registered - routes := r.Routes() - found := false - for _, route := range routes { - if route.Path == "/docker/containers" && route.Method == "GET" { - found = true - break - } - } - assert.True(t, found, "Expected /docker/containers GET route to be registered") -} - -func TestDockerHandler_NewDockerHandler(t *testing.T) { - svc, _ := services.NewDockerService() - if svc == nil { - t.Skip("Docker not available") - } - - _, _, rsService := setupDockerTestRouter(t) - - h := NewDockerHandler(svc, rsService) - assert.NotNil(t, h) - assert.NotNil(t, h.dockerService) - assert.NotNil(t, h.remoteServerService) + assert.False(t, dockerSvc.called) } diff --git a/backend/internal/api/routes/routes.go b/backend/internal/api/routes/routes.go index 466dd5a1..0b15789c 100644 --- a/backend/internal/api/routes/routes.go +++ b/backend/internal/api/routes/routes.go @@ -453,7 +453,7 @@ func Register(router *gin.Engine, db *gorm.DB, cfg config.Config) error { // Caddy Manager already created above proxyHostHandler := handlers.NewProxyHostHandler(db, caddyManager, notificationService, uptimeService) - proxyHostHandler.RegisterRoutes(api) + proxyHostHandler.RegisterRoutes(protected) remoteServerHandler := handlers.NewRemoteServerHandler(remoteServerService, notificationService) remoteServerHandler.RegisterRoutes(api) diff --git a/backend/internal/api/routes/routes_test.go b/backend/internal/api/routes/routes_test.go index 8ee836ae..72ae3b45 100644 --- a/backend/internal/api/routes/routes_test.go +++ b/backend/internal/api/routes/routes_test.go @@ -1,6 +1,9 @@ package routes import ( + "net/http" + "net/http/httptest" + "strings" "testing" "github.com/Wikid82/charon/backend/internal/config" @@ -151,3 +154,23 @@ func TestRegister_RoutesRegistration(t *testing.T) { assert.True(t, routeMap[expected], "Route %s should be registered", expected) } } + +func TestRegister_ProxyHostsRequireAuth(t *testing.T) { + gin.SetMode(gin.TestMode) + router := gin.New() + + // Use in-memory DB + db, err := gorm.Open(sqlite.Open("file::memory:?cache=shared&_test_proxyhosts_auth"), &gorm.Config{}) + require.NoError(t, err) + + cfg := config.Config{JWTSecret: "test-secret"} + require.NoError(t, Register(router, db, cfg)) + + req := httptest.NewRequest(http.MethodPost, "/api/v1/proxy-hosts", strings.NewReader(`{}`)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusUnauthorized, w.Code) + assert.Contains(t, w.Body.String(), "Authorization header required") +} diff --git a/backend/internal/services/docker_service.go b/backend/internal/services/docker_service.go index 2a222717..04094d89 100644 --- a/backend/internal/services/docker_service.go +++ b/backend/internal/services/docker_service.go @@ -2,14 +2,41 @@ package services import ( "context" + "errors" "fmt" + "net" + "net/url" + "os" "strings" + "syscall" "github.com/Wikid82/charon/backend/internal/logger" "github.com/docker/docker/api/types/container" "github.com/docker/docker/client" ) +type DockerUnavailableError struct { + err error +} + +func NewDockerUnavailableError(err error) *DockerUnavailableError { + return &DockerUnavailableError{err: err} +} + +func (e *DockerUnavailableError) Error() string { + if e == nil || e.err == nil { + return "docker unavailable" + } + return fmt.Sprintf("docker unavailable: %v", e.err) +} + +func (e *DockerUnavailableError) Unwrap() error { + if e == nil { + return nil + } + return e.err +} + type DockerPort struct { PrivatePort uint16 `json:"private_port"` PublicPort uint16 `json:"public_port"` @@ -59,6 +86,9 @@ func (s *DockerService) ListContainers(ctx context.Context, host string) ([]Dock containers, err := cli.ContainerList(ctx, container.ListOptions{All: false}) if err != nil { + if isDockerConnectivityError(err) { + return nil, &DockerUnavailableError{err: err} + } return nil, fmt.Errorf("failed to list containers: %w", err) } @@ -105,3 +135,60 @@ func (s *DockerService) ListContainers(ctx context.Context, host string) ([]Dock return result, nil } + +func isDockerConnectivityError(err error) bool { + if err == nil { + return false + } + + // Common high-signal strings from docker client/daemon failures. + msg := strings.ToLower(err.Error()) + if strings.Contains(msg, "cannot connect to the docker daemon") || + strings.Contains(msg, "is the docker daemon running") || + strings.Contains(msg, "error during connect") { + return true + } + + // Context timeouts typically indicate the daemon/socket is unreachable. + if errors.Is(err, context.DeadlineExceeded) { + return true + } + + var urlErr *url.Error + if errors.As(err, &urlErr) { + err = urlErr.Unwrap() + } + + var netErr net.Error + if errors.As(err, &netErr) { + if netErr.Timeout() { + return true + } + } + + // Walk common syscall error wrappers. + var syscallErr *os.SyscallError + if errors.As(err, &syscallErr) { + err = syscallErr.Unwrap() + } + + var opErr *net.OpError + if errors.As(err, &opErr) { + err = opErr.Unwrap() + } + + var errno syscall.Errno + if errors.As(err, &errno) { + switch errno { + case syscall.ENOENT, syscall.EACCES, syscall.EPERM, syscall.ECONNREFUSED: + return true + } + } + + // os.ErrNotExist covers missing unix socket paths. + if errors.Is(err, os.ErrNotExist) { + return true + } + + return false +} diff --git a/docs/plans/docker_socket_trace.md b/docs/plans/docker_socket_trace.md new file mode 100644 index 00000000..23bd9249 --- /dev/null +++ b/docs/plans/docker_socket_trace.md @@ -0,0 +1,362 @@ +# Docker Socket Trace Analysis + +**Date**: 2025-12-22 +**Issue**: Creating a new proxy host using the local docker socket fails with 503 (previously 500) +**Status**: Root cause identified + +--- + +## Executive Summary + +**ROOT CAUSE**: The container runs as non-root user `charon` (uid=1000, gid=1000), but the Docker socket mounted into the container is owned by `root:docker` (gid=988 on host). The `charon` user is not a member of the `docker` group, so socket access is denied with `Permission denied`. + +**The 503 is correct behavior** - it accurately reflects that Docker is unavailable due to permission restrictions. The error handling code change from 500 to 503 was an improvement, not a bug. + +--- + +## 1. Full Workflow Trace + +### Frontend Layer + +#### A. ProxyHostForm Component +- **File**: [frontend/src/components/ProxyHostForm.tsx](../../frontend/src/components/ProxyHostForm.tsx) +- **State**: `connectionSource` - defaults to `'custom'`, can be `'local'` or a remote server UUID +- **Hook invocation** (line ~146): + ```typescript + const { containers: dockerContainers, isLoading: dockerLoading, error: dockerError } = useDocker( + connectionSource === 'local' ? 'local' : undefined, + connectionSource !== 'local' && connectionSource !== 'custom' ? connectionSource : undefined + ) + ``` +- **Error display** (line ~361): + ```typescript + {dockerError && connectionSource !== 'custom' && ( +

+ Failed to connect: {(dockerError as Error).message} +

+ )} + ``` + +#### B. useDocker Hook +- **File**: [frontend/src/hooks/useDocker.ts](../../frontend/src/hooks/useDocker.ts) +- **Function**: `useDocker(host?: string | null, serverId?: string | null)` +- **Query configuration**: + ```typescript + useQuery({ + queryKey: ['docker-containers', host, serverId], + queryFn: () => dockerApi.listContainers(host || undefined, serverId || undefined), + enabled: Boolean(host) || Boolean(serverId), + retry: 1, + }) + ``` +- When `connectionSource === 'local'`, calls `dockerApi.listContainers('local', undefined)` + +#### C. Docker API Client +- **File**: [frontend/src/api/docker.ts](../../frontend/src/api/docker.ts) +- **Function**: `dockerApi.listContainers(host?: string, serverId?: string)` +- **Request**: `GET /api/v1/docker/containers?host=local` +- **Response type**: `DockerContainer[]` + +--- + +### Backend Layer + +#### D. Routes Registration +- **File**: [backend/internal/api/routes/routes.go](../../backend/internal/api/routes/routes.go) +- **Registration** (lines 199-204): + ```go + dockerService, err := services.NewDockerService() + if err == nil { // Only register if Docker is available + dockerHandler := handlers.NewDockerHandler(dockerService, remoteServerService) + dockerHandler.RegisterRoutes(protected) + } else { + logger.Log().WithError(err).Warn("Docker service unavailable") + } + ``` +- **CRITICAL**: Docker routes only register if `NewDockerService()` succeeds (client construction, not socket access) +- Route: `GET /api/v1/docker/containers` (protected, requires auth) + +#### E. Docker Handler +- **File**: [backend/internal/api/handlers/docker_handler.go](../../backend/internal/api/handlers/docker_handler.go) +- **Function**: `ListContainers(c *gin.Context)` +- **Input validation** (SSRF hardening): + ```go + host := strings.TrimSpace(c.Query("host")) + serverID := strings.TrimSpace(c.Query("server_id")) + + // SSRF hardening: only allow "local" or empty + if host != "" && host != "local" { + c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid docker host selector"}) + return + } + ``` +- **Service call**: `h.dockerService.ListContainers(c.Request.Context(), host)` +- **Error handling** (lines 60-69): + ```go + if err != nil { + var unavailableErr *services.DockerUnavailableError + if errors.As(err, &unavailableErr) { + c.JSON(http.StatusServiceUnavailable, gin.H{"error": "Docker daemon unavailable"}) // 503 + return + } + c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to list containers"}) // 500 + return + } + ``` + +#### F. Docker Service +- **File**: [backend/internal/services/docker_service.go](../../backend/internal/services/docker_service.go) +- **Constructor**: `NewDockerService()` + ```go + cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) + ``` + - Uses `client.FromEnv` which reads `DOCKER_HOST` env var (defaults to `unix:///var/run/docker.sock`) + - **Does NOT verify socket access** - only constructs client object + +- **Function**: `ListContainers(ctx context.Context, host string)` + ```go + if host == "" || host == "local" { + cli = s.client // Use default local client + } + containers, err := cli.ContainerList(ctx, container.ListOptions{All: false}) + if err != nil { + if isDockerConnectivityError(err) { + return nil, &DockerUnavailableError{err: err} // Triggers 503 + } + return nil, fmt.Errorf("failed to list containers: %w", err) // Triggers 500 + } + ``` + +- **Error detection**: `isDockerConnectivityError(err)` (lines 104-152) + - Checks for: "cannot connect to docker daemon", "is the docker daemon running", timeout errors + - Checks syscall errors: `ENOENT`, `EACCES`, `EPERM`, `ECONNREFUSED` + - **Matches `syscall.EACCES` (permission denied)** → returns `DockerUnavailableError` → **503** + +--- + +## 2. Request/Response Shapes + +### Frontend → Backend Request +``` +GET /api/v1/docker/containers?host=local +Authorization: Bearer +``` + +### Backend → Frontend Response (Success - 200) +```json +[ + { + "id": "abc123def456", + "names": ["my-container"], + "image": "nginx:latest", + "state": "running", + "status": "Up 2 hours", + "network": "bridge", + "ip": "172.17.0.2", + "ports": [{"private_port": 80, "public_port": 8080, "type": "tcp"}] + } +] +``` + +### Backend → Frontend Response (Error - 503) +```json +{ + "error": "Docker daemon unavailable" +} +``` + +--- + +## 3. Error Conditions Triggering 503 + +The 503 `Service Unavailable` is returned when `isDockerConnectivityError()` returns `true`: + +| Condition | Check in Code | Matches Our Case | +|-----------|---------------|------------------| +| Socket missing | `syscall.ENOENT` or `os.ErrNotExist` | No | +| Permission denied | `syscall.EACCES` or `syscall.EPERM` | **YES** ✓ | +| Connection refused | `syscall.ECONNREFUSED` | No | +| Timeout | `net.Error.Timeout()` or `context.DeadlineExceeded` | No | +| Daemon not running | String contains "cannot connect" / "daemon running" | No | + +--- + +## 4. Docker Configuration Analysis + +### Dockerfile +- **File**: [Dockerfile](../../Dockerfile) +- **User creation** (lines 154-156): + ```dockerfile + RUN addgroup -g 1000 charon && \ + adduser -D -u 1000 -G charon -h /app -s /sbin/nologin charon + ``` +- **Runtime user** (line 286): + ```dockerfile + USER charon + ``` +- **Result**: Container runs as `uid=1000, gid=1000` (charon:charon) + +### Docker Compose Files +All compose files mount the socket identically: +```yaml +volumes: + - /var/run/docker.sock:/var/run/docker.sock:ro +``` + +| File | Mount Present | +|------|---------------| +| [.docker/compose/docker-compose.yml](../../.docker/compose/docker-compose.yml) | ✓ | +| [.docker/compose/docker-compose.local.yml](../../.docker/compose/docker-compose.local.yml) | ✓ | +| [.docker/compose/docker-compose.dev.yml](../../.docker/compose/docker-compose.dev.yml) | ✓ | +| [docker-compose.test.yml](../../docker-compose.test.yml) | ✓ | + +### Runtime Verification (from live container) + +```bash +# Socket exists inside container +$ ls -la /var/run/docker.sock +srw-rw---- 1 root 988 0 Dec 12 22:40 /var/run/docker.sock + +# Container user identity +$ id +uid=1000(charon) gid=1000(charon) groups=1000(charon) + +# Direct socket access test +$ curl --unix-socket /var/run/docker.sock http://localhost/containers/json +# Returns: exit code 7 (connection refused due to permission denied) + +# Explicit permission check +$ cat /var/run/docker.sock +cat: can't open '/var/run/docker.sock': Permission denied +``` + +### Host System +```bash +$ getent group 988 +docker:x:988: + +$ stat -c '%U:%G' /var/run/docker.sock +root:docker +``` + +--- + +## 5. Root Cause Analysis + +### The Permission Gap + +| Component | Value | +|-----------|-------| +| Socket owner | `root:docker` (gid=988) | +| Socket permissions | `srw-rw----` (660) | +| Container user | `charon` (uid=1000, gid=1000) | +| Container groups | Only `charon` (1000) | +| Docker group in container | **Does not exist** | + +**The `charon` user cannot access the socket because:** +1. Not owner (not root) +2. Not in the socket's group (gid=988 doesn't exist in container, and charon isn't in it) +3. No "other" permissions on socket + +### Why This Happens + +The Docker socket's group ID (988 on this host) is a **host-specific value**. Different systems assign different GIDs to the `docker` group: +- Debian/Ubuntu: often 999 or 998 +- Alpine: often 101 (from `docker` package) +- RHEL/CentOS: varies +- This host: 988 + +The container has no knowledge of the host's group mappings. When the socket is mounted, it retains the host's numeric GID, but the container has no group with that GID. + +--- + +## 6. Why 503 (Not 500) Is Correct + +The error mapping change that returned 503 instead of 500 was **correct and intentional**: + +- **500 Internal Server Error**: Indicates a bug or unexpected failure in the application +- **503 Service Unavailable**: Indicates the requested service is temporarily unavailable due to external factors + +Docker being inaccessible due to socket permissions is an **environmental/configuration issue**, not an application bug. The 503 correctly signals: +1. The API endpoint is working +2. The underlying Docker service is unavailable +3. The issue is likely external (deployment configuration) + +--- + +## 7. Solutions + +### Option A: Run Container as Root (Not Recommended) +Remove `USER charon` from Dockerfile. Breaks security best practices (CIS Docker Benchmark 4.1). + +### Option B: Add Docker Group to Container at Build Time +```dockerfile +# Problem: GID varies by host system +RUN addgroup -g 988 docker && adduser charon docker +``` +**Issue**: Assumes host Docker GID is 988; breaks on other systems. + +### Option C: Dynamic Group Assignment at Runtime (Recommended) +Modify entrypoint to detect and add the socket's group: + +```bash +# In docker-entrypoint.sh, before starting the app: +if [ -S /var/run/docker.sock ]; then + DOCKER_GID=$(stat -c '%g' /var/run/docker.sock) + if ! getent group "$DOCKER_GID" >/dev/null 2>&1; then + # Create a group with the socket's GID + addgroup -g "$DOCKER_GID" docker 2>/dev/null || true + fi + # Add charon user to the docker group + adduser charon docker 2>/dev/null || true +fi +``` + +**Issue**: Requires container to start as root, then drop privileges. + +### Option D: Use DOCKER_HOST Environment Variable +Allow users to specify an alternative Docker endpoint (TCP, SSH, or different socket path): +```yaml +environment: + - DOCKER_HOST=tcp://host.docker.internal:2375 +``` +**Issue**: Requires exposing Docker API over network (security implications). + +### Option E: Document User Requirement (Workaround) +Add documentation requiring users to either: +1. Run the container with `--user root` (not recommended) +2. Change socket permissions on host: `chmod 666 /var/run/docker.sock` (security risk) +3. Accept that Docker integration is unavailable when running as non-root + +--- + +## 8. Recommendations + +### Immediate (No Code Change) +1. **Update documentation** to explain the permission requirement +2. **Add health check** for Docker availability in the UI (show "Docker integration unavailable" gracefully) + +### Short Term +1. **Add startup warning log** when Docker socket is inaccessible: + ```go + // In routes.go or docker_service.go + if _, err := cli.Ping(ctx); err != nil { + logger.Log().Warn("Docker socket inaccessible - container discovery disabled") + } + ``` + +### Medium Term +1. **Implement Option C** with proper privilege dropping +2. **Add environment variable** `CHARON_DOCKER_ENABLED=false` to explicitly disable Docker integration + +### Long Term +1. Consider **podman socket** compatibility +2. Consider **Docker SDK over TCP** as alternative + +--- + +## 9. Conclusion + +The 503 error is **working as designed**. The Docker socket permission model fundamentally conflicts with running containers as non-root users unless explicit configuration is done at deployment time. + +**The fix is not in the code, but in deployment configuration or documentation.** diff --git a/docs/plans/notification_page_trace.md b/docs/plans/notification_page_trace.md new file mode 100644 index 00000000..81a6cf01 --- /dev/null +++ b/docs/plans/notification_page_trace.md @@ -0,0 +1,303 @@ +# Notification Page Trace Analysis + +**Date**: 2025-12-22 +**Issue**: User moved notification page in layout.tsx but can't find the rest of the code to render it correctly +**Status**: Routing mismatch identified + +--- + +## Executive Summary + +**ROOT CAUSE**: There is a **routing mismatch** between the navigation link and the actual React Router route definition: + +| Component | Path | Status | +|-----------|------|--------| +| **Layout.tsx** navigation | `/settings/notifications` | ✅ Points here | +| **App.tsx** route | `/notifications` | ❌ Route is defined here | +| **Settings.tsx** tabs | Missing notifications tab | ❌ Not integrated | + +**Result**: Clicking "Notifications" in the sidebar navigates to `/settings/notifications`, but there is **no route defined for that path**. The route exists at `/notifications` (top-level), not as a nested settings route. + +--- + +## 1. Full Workflow Mapping + +### Frontend Layer + +#### A. Layout.tsx (Navigation Definition) +- **File**: [frontend/src/components/Layout.tsx](../../frontend/src/components/Layout.tsx#L81) +- **Navigation entry** (line 81): + ```typescript + { name: t('navigation.notifications'), path: '/settings/notifications', icon: '🔔' }, + ``` +- **Location**: Nested under the "Settings" menu group +- **Status**: ✅ Entry exists, links to `/settings/notifications` + +#### B. App.tsx (Route Definitions) +- **File**: [frontend/src/App.tsx](../../frontend/src/App.tsx) +- **Notifications route** (line 70): + ```typescript + } /> + ``` +- **Location**: Top-level route under the authenticated layout (NOT under `/settings`) +- **Actual path**: `/notifications` +- **Status**: ⚠️ Route exists at WRONG path + +#### C. Settings.tsx (Settings Tab Navigation) +- **File**: [frontend/src/pages/Settings.tsx](../../frontend/src/pages/Settings.tsx) +- **Tab items** (lines 14-18): + ```typescript + const navItems = [ + { path: '/settings/system', label: t('settings.system'), icon: Server }, + { path: '/settings/smtp', label: t('settings.smtp'), icon: Mail }, + { path: '/settings/account', label: t('settings.account'), icon: User }, + ] + ``` +- **Status**: ❌ **Missing notifications tab** - not integrated into Settings page + +#### D. Notifications.tsx (Page Component) +- **File**: [frontend/src/pages/Notifications.tsx](../../frontend/src/pages/Notifications.tsx) +- **Status**: ✅ **Fully implemented** - manages notification providers, templates, tests +- **Features**: + - Provider CRUD (Discord, Slack, Gotify, Telegram, Generic/Custom Webhook) + - External template management + - Provider testing + - Provider preview + - Event type subscriptions (proxy hosts, remote servers, domains, certs, uptime) + +#### E. useNotifications.ts (Hook) +- **File**: [frontend/src/hooks/useNotifications.ts](../../frontend/src/hooks/useNotifications.ts) +- **Purpose**: Security notification settings (different from provider management) +- **Hooks exported**: + - `useSecurityNotificationSettings()` - fetches security notification config + - `useUpdateSecurityNotificationSettings()` - updates security notification config +- **API endpoints used**: + - `GET /api/v1/notifications/settings/security` + - `PUT /api/v1/notifications/settings/security` +- **Note**: This hook is for **security-specific** notifications (WAF, ACL, rate limiting), NOT the general notification providers page + +#### F. NotificationCenter.tsx (Header Component) +- **File**: [frontend/src/components/NotificationCenter.tsx](../../frontend/src/components/NotificationCenter.tsx) +- **Purpose**: Dropdown bell icon in header showing system notifications +- **API endpoints used**: + - `GET /api/v1/notifications` (from `api/system.ts`) + - `POST /api/v1/notifications/:id/read` + - `POST /api/v1/notifications/read-all` + - `GET /api/v1/system/updates` +- **Status**: ✅ Working correctly, separate from the settings page + +#### G. API Client - notifications.ts +- **File**: [frontend/src/api/notifications.ts](../../frontend/src/api/notifications.ts) +- **Exports**: + - Provider CRUD: `getProviders`, `createProvider`, `updateProvider`, `deleteProvider`, `testProvider` + - Templates: `getTemplates`, `getExternalTemplates`, `createExternalTemplate`, `updateExternalTemplate`, `deleteExternalTemplate`, `previewExternalTemplate` + - Provider preview: `previewProvider` + - Security settings: `getSecurityNotificationSettings`, `updateSecurityNotificationSettings` +- **Status**: ✅ Complete + +--- + +### Backend Layer + +#### H. routes.go (Route Registration) +- **File**: [backend/internal/api/routes/routes.go](../../backend/internal/api/routes/routes.go) +- **Notification endpoints registered**: + +| Endpoint | Handler | Line | +|----------|---------|------| +| `GET /notifications` | `notificationHandler.List` | 232 | +| `POST /notifications/:id/read` | `notificationHandler.MarkAsRead` | 233 | +| `POST /notifications/read-all` | `notificationHandler.MarkAllAsRead` | 234 | +| `GET /notifications/providers` | `notificationProviderHandler.List` | 269 | +| `POST /notifications/providers` | `notificationProviderHandler.Create` | 270 | +| `PUT /notifications/providers/:id` | `notificationProviderHandler.Update` | 271 | +| `DELETE /notifications/providers/:id` | `notificationProviderHandler.Delete` | 272 | +| `POST /notifications/providers/test` | `notificationProviderHandler.Test` | 273 | +| `POST /notifications/providers/preview` | `notificationProviderHandler.Preview` | 274 | +| `GET /notifications/templates` | `notificationProviderHandler.Templates` | 275 | +| `GET /notifications/external-templates` | `notificationTemplateHandler.List` | 278 | +| `POST /notifications/external-templates` | `notificationTemplateHandler.Create` | 279 | +| `PUT /notifications/external-templates/:id` | `notificationTemplateHandler.Update` | 280 | +| `DELETE /notifications/external-templates/:id` | `notificationTemplateHandler.Delete` | 281 | +| `POST /notifications/external-templates/preview` | `notificationTemplateHandler.Preview` | 282 | +| `GET /security/notifications/settings` | `securityNotificationHandler.GetSettings` | 180 | +| `PUT /security/notifications/settings` | `securityNotificationHandler.UpdateSettings` | 181 | + +- **Status**: ✅ All backend routes exist + +#### I. Handler Files +- `notification_handler.go` - System notifications list/read +- `notification_provider_handler.go` - Provider CRUD +- `notification_template_handler.go` - External templates +- `security_notifications.go` - Security-specific notification settings +- **Status**: ✅ All handlers implemented + +#### J. Service Files +- `notification_service.go` - Core notification service +- `security_notification_service.go` - Security notification config +- **Status**: ✅ All services implemented + +#### K. Model Files +- `notification.go` - System notification model +- `notification_provider.go` - Provider model +- `notification_template.go` - Template model +- `notification_config.go` - Config model +- **Status**: ✅ All models defined + +--- + +## 2. What's MISSING or BROKEN + +### Critical Issue: Route Mismatch + +| Issue | Description | Impact | +|-------|-------------|--------| +| **Route path mismatch** | Layout links to `/settings/notifications` but route is at `/notifications` | Page not found when clicking nav | +| **Settings integration missing** | Settings.tsx doesn't include notifications tab | Even if route fixed, no tab in settings UI | +| **Route nesting incorrect** | Route defined at top level, not under `/settings/*` | Inconsistent with navigation structure | + +### Missing Integration + +The Notifications page component exists and is fully functional, but it's **not wired up correctly** to the navigation: + +``` +User clicks "Notifications" in sidebar + ↓ +Navigation points to: /settings/notifications + ↓ +App.tsx has NO route for /settings/notifications + ↓ +React Router shows: blank content (falls through to no match) +``` + +--- + +## 3. Recent Git Changes + +From `get_changed_files`, the relevant recent change to Layout.tsx: + +```diff +- { name: t('navigation.notifications'), path: '/notifications', icon: '🔔' }, +- // Import group moved under Tasks ++ { name: t('navigation.notifications'), path: '/settings/notifications', icon: '🔔' }, +``` + +**What happened**: The navigation path was changed from `/notifications` (which matches the route) to `/settings/notifications` (which has no route), and the entry was moved under the Settings submenu. + +**The route in App.tsx was NOT updated** to match this change. + +--- + +## 4. Two Distinct Notification Features + +There are actually **TWO different notification features** that may be causing confusion: + +### Feature 1: Notification Providers (Settings Page) +- **Purpose**: Configure external notification channels (Discord, Slack, etc.) +- **Page**: `Notifications.tsx` +- **API**: `/api/v1/notifications/providers/*`, `/api/v1/notifications/external-templates/*` +- **This is what the settings navigation should show** + +### Feature 2: System Notifications (Header Bell) +- **Purpose**: In-app notification center showing system events +- **Component**: `NotificationCenter.tsx` +- **API**: `/api/v1/notifications`, `/api/v1/notifications/:id/read` +- **This already works correctly in the header** + +### Feature 3: Security Notifications (Cerberus Modal) +- **Purpose**: Configure notifications for security events (WAF blocks, ACL denials, etc.) +- **Component**: `SecurityNotificationSettingsModal.tsx` +- **Hook**: `useNotifications.ts` +- **API**: `/api/v1/security/notifications/settings` +- **Accessed from Cerberus/Security dashboard** + +--- + +## 5. Solutions + +### Option A: Fix Route to Match Navigation (Recommended) + +Update App.tsx to add the route under settings: + +```typescript +{/* Settings Routes */} +}> + } /> + } /> + } /> // ADD THIS + } /> + } /> + } /> + +``` + +Also update Settings.tsx to add the tab: + +```typescript +const navItems = [ + { path: '/settings/system', label: t('settings.system'), icon: Server }, + { path: '/settings/notifications', label: t('settings.notifications'), icon: Bell }, // ADD THIS + { path: '/settings/smtp', label: t('settings.smtp'), icon: Mail }, + { path: '/settings/account', label: t('settings.account'), icon: User }, +] +``` + +### Option B: Revert Navigation to Match Route + +Change Layout.tsx back to use the existing route: + +```typescript +// Move outside settings submenu, at top level +{ name: t('navigation.notifications'), path: '/notifications', icon: '🔔' }, +``` + +This keeps the existing route but changes the navigation structure. + +--- + +## 6. Test Files Review + +### Existing Tests (All Pass) + +| Test File | Coverage | +|-----------|----------| +| `useNotifications.test.tsx` | Security notification hooks ✅ | +| `NotificationCenter.test.tsx` | Header notification dropdown ✅ | +| `SecurityNotificationSettingsModal.test.tsx` | Security settings modal ✅ | +| `notification_handler_test.go` | System notifications API ✅ | +| `notification_provider_handler_test.go` | Provider API ✅ | +| `notification_template_handler_test.go` | Template API ✅ | +| `security_notifications_test.go` | Security notifications API ✅ | + +**No tests would prevent the routing fix** - the tests cover API and component behavior, not navigation routing. + +--- + +## 7. Summary + +### What EXISTS and WORKS: +- ✅ `Notifications.tsx` page component (fully implemented) +- ✅ `notifications.ts` API client (complete) +- ✅ Backend handlers and routes (complete) +- ✅ Database models (complete) +- ✅ `NotificationCenter.tsx` header component (works) +- ✅ Navigation link in Layout.tsx (points to `/settings/notifications`) + +### What's BROKEN: +- ❌ **Route definition** - Route is at `/notifications` but navigation points to `/settings/notifications` +- ❌ **Settings.tsx tabs** - Missing notifications tab + +### What NEEDS to be done: +1. Add route `} />` under `/settings/*` in App.tsx +2. Add notifications tab to Settings.tsx navItems array +3. Optionally remove the old `/notifications` top-level route to avoid confusion + +--- + +## 8. Quick Fix Checklist + +- [ ] In `App.tsx`: Add `} />` inside the `` block +- [ ] In `Settings.tsx`: Add `{ path: '/settings/notifications', label: t('settings.notifications'), icon: Bell }` to navItems +- [ ] In `Settings.tsx`: Import `Bell` from lucide-react +- [ ] Optional: Remove `} />` from top level in App.tsx (line 70) +- [ ] Test: Navigate to Settings → Notifications tab should appear and work diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 5b8a98d4..4fa6832d 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -67,7 +67,6 @@ export default function App() { } /> } /> } /> - } /> } /> } /> @@ -75,6 +74,7 @@ export default function App() { }> } /> } /> + } /> } /> } /> } /> diff --git a/frontend/src/components/Layout.tsx b/frontend/src/components/Layout.tsx index 5e5c7122..a14784a3 100644 --- a/frontend/src/components/Layout.tsx +++ b/frontend/src/components/Layout.tsx @@ -72,14 +72,13 @@ export default function Layout({ children }: LayoutProps) { { name: t('navigation.waf'), path: '/security/waf', icon: '🛡️' }, { name: t('navigation.securityHeaders'), path: '/security/headers', icon: '🔐' }, ]}, - { name: t('navigation.notifications'), path: '/notifications', icon: '🔔' }, - // Import group moved under Tasks { name: t('navigation.settings'), path: '/settings', icon: '⚙️', children: [ { name: t('navigation.system'), path: '/settings/system', icon: '⚙️' }, + { name: t('navigation.notifications'), path: '/settings/notifications', icon: '🔔' }, { name: t('navigation.email'), path: '/settings/smtp', icon: '📧' }, { name: t('navigation.adminAccount'), path: '/settings/account', icon: '🛡️' }, { name: t('navigation.accountManagement'), path: '/settings/account-management', icon: '👥' }, @@ -93,7 +92,6 @@ export default function Layout({ children }: LayoutProps) { { name: t('navigation.import'), path: '/tasks/import', - icon: '📥', children: [ { name: t('navigation.caddyfile'), path: '/tasks/import/caddyfile', icon: '📥' }, { name: t('navigation.crowdsec'), path: '/tasks/import/crowdsec', icon: '🛡️' }, diff --git a/frontend/src/hooks/__tests__/useDocker.test.tsx b/frontend/src/hooks/__tests__/useDocker.test.tsx index c7b71443..1e14f39a 100644 --- a/frontend/src/hooks/__tests__/useDocker.test.tsx +++ b/frontend/src/hooks/__tests__/useDocker.test.tsx @@ -81,6 +81,15 @@ describe('useDocker', () => { expect(result.current.containers).toEqual([]); }); + it('does not fetch when both host and serverId are undefined', async () => { + const { result } = renderHook(() => useDocker(undefined, undefined), { + wrapper: createWrapper(), + }); + + expect(dockerApi.listContainers).not.toHaveBeenCalled(); + expect(result.current.containers).toEqual([]); + }); + it('returns empty array as default when no data', async () => { vi.mocked(dockerApi.listContainers).mockResolvedValue([]); diff --git a/frontend/src/hooks/useDocker.ts b/frontend/src/hooks/useDocker.ts index 6416b499..4c8f01a0 100644 --- a/frontend/src/hooks/useDocker.ts +++ b/frontend/src/hooks/useDocker.ts @@ -10,7 +10,7 @@ export function useDocker(host?: string | null, serverId?: string | null) { } = useQuery({ queryKey: ['docker-containers', host, serverId], queryFn: () => dockerApi.listContainers(host || undefined, serverId || undefined), - enabled: host !== null || serverId !== null, // Disable if both are explicitly null/undefined + enabled: Boolean(host) || Boolean(serverId), retry: 1, // Don't retry too much if docker is not available }) diff --git a/frontend/src/pages/Settings.tsx b/frontend/src/pages/Settings.tsx index 158f6dc3..3b6bb10e 100644 --- a/frontend/src/pages/Settings.tsx +++ b/frontend/src/pages/Settings.tsx @@ -2,7 +2,7 @@ import { Link, Outlet, useLocation } from 'react-router-dom' import { useTranslation } from 'react-i18next' import { PageShell } from '../components/layout/PageShell' import { cn } from '../utils/cn' -import { Settings as SettingsIcon, Server, Mail, User } from 'lucide-react' +import { Settings as SettingsIcon, Server, Mail, User, Bell } from 'lucide-react' export default function Settings() { const { t } = useTranslation() @@ -12,6 +12,7 @@ export default function Settings() { const navItems = [ { path: '/settings/system', label: t('settings.system'), icon: Server }, + { path: '/settings/notifications', label: t('settings.notifications'), icon: Bell }, { path: '/settings/smtp', label: t('settings.smtp'), icon: Mail }, { path: '/settings/account', label: t('settings.account'), icon: User }, ]