Skip to content

Commit bacd807

Browse files
authored
fix(dashboard): report genuine Docker daemon availability in /api/v1/docker/status (MCP-2478) (#683)
Root cause: GetDockerRecoveryStatus() short-circuits when Docker recovery is disabled (isolation off) and returns synthetic DockerAvailable:true. The dashboard bound its 'Docker isolation active' badge to docker_available, so hosts with no Docker daemon and isolation disabled showed 'active'. Backend: - Add IsDockerAvailable() to ServerController interface; delegates to Runtime.IsDockerAvailable() (the same time-cached probe telemetry uses). - /api/v1/docker/status now reports the genuine probe result instead of the synthetic recovery-state value. - Add isolation_enabled field (config docker_isolation.enabled) so the UI can check both conditions independently. Frontend (Dashboard.vue): - Show 'active' only when docker_available && isolation_enabled. - Remove the :538 stdio-connected workaround that faked availability. TDD: docker_status_test.go covers all three acceptance cases: no-Docker+isolation-off -> disabled Docker+isolation-on -> active stdio-connected != active
1 parent 38036ea commit bacd807

8 files changed

Lines changed: 172 additions & 10 deletions

File tree

docs/api/rest-api.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,19 @@ MCPProxy automatically checks for updates every 4 hours. The update information
745745

746746
Get Docker isolation status.
747747

748+
**Response fields:**
749+
750+
| Field | Type | Description |
751+
|-------|------|-------------|
752+
| `docker_available` | bool | Genuine Docker daemon reachability (result of a real `docker info` probe). |
753+
| `isolation_enabled` | bool | Whether `docker_isolation.enabled` is set in config. The UI treats isolation as "active" only when both this and `docker_available` are true. |
754+
| `recovery_mode` | bool | Whether the Docker recovery monitor is actively retrying. |
755+
| `failure_count` | int | Consecutive recovery failures. |
756+
| `attempts_since_up` | int | Recovery attempts since the daemon was last seen available. |
757+
| `last_attempt` | string | Timestamp of the last recovery attempt. |
758+
| `last_error` | string | Last recovery error message, if any. |
759+
| `last_successful_at` | string | Timestamp of the last successful daemon contact. |
760+
748761
### Secrets
749762

750763
#### GET /api/v1/secrets

frontend/src/services/api.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,7 @@ class APIService {
510510
// Docker status
511511
async getDockerStatus(): Promise<APIResponse<{
512512
docker_available: boolean
513+
isolation_enabled: boolean
513514
recovery_mode: boolean
514515
failure_count: number
515516
attempts_since_up: number

frontend/src/views/Dashboard.vue

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -528,17 +528,16 @@ const {
528528
529529
const loadSecurityStatus = async () => {
530530
try {
531-
// Docker status from dedicated endpoint
531+
// Docker status from dedicated endpoint. The badge reads "active" only when
532+
// Docker isolation is genuinely in effect: the user enabled it AND a real
533+
// Docker daemon is reachable. docker_available now reflects a genuine daemon
534+
// probe (MCP-2478) — we must NOT fake it from connected stdio servers, which
535+
// do not imply isolation is on.
532536
const dockerResponse = await api.getDockerStatus()
533537
if (dockerResponse.success && dockerResponse.data) {
534-
let available = dockerResponse.data.docker_available ?? false
535-
// Workaround: Docker health checker can get stuck at 'max retries exceeded'
536-
// even when Docker containers are running. If API says unavailable but
537-
// we have connected stdio servers (which use Docker), treat as available.
538-
if (!available && serversStore.servers.some(s => s.connected && s.protocol === 'stdio')) {
539-
available = true
540-
}
541-
dockerStatus.value = { available }
538+
const daemonAvailable = dockerResponse.data.docker_available ?? false
539+
const isolationEnabled = dockerResponse.data.isolation_enabled ?? false
540+
dockerStatus.value = { available: daemonAvailable && isolationEnabled }
542541
}
543542
} catch {
544543
// Docker endpoint may not exist - treat as unavailable

internal/httpapi/contracts_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ func (m *MockServerController) GetDockerRecoveryStatus() *storage.DockerRecovery
168168
LastError: "",
169169
}
170170
}
171+
func (m *MockServerController) IsDockerAvailable() bool { return true }
171172
func (m *MockServerController) GetRecentSessions(_ int) ([]*contracts.MCPSession, int, error) {
172173
return []*contracts.MCPSession{}, 0, nil
173174
}
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
package httpapi
2+
3+
import (
4+
"encoding/json"
5+
"net/http"
6+
"net/http/httptest"
7+
"testing"
8+
9+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
10+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/storage"
11+
12+
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/require"
14+
"go.uber.org/zap"
15+
)
16+
17+
// mockDockerStatusController lets a test drive the genuine daemon probe and the
18+
// configured docker_isolation.enabled flag independently, while keeping
19+
// GetDockerRecoveryStatus returning the synthetic DockerAvailable:true that
20+
// production returns when Docker recovery is disabled (isolation off). This
21+
// proves /api/v1/docker/status reports the GENUINE probe value, not the
22+
// synthetic recovery-state value (MCP-2478).
23+
type mockDockerStatusController struct {
24+
baseController
25+
apiKey string
26+
dockerAvailable bool
27+
isolationEnabled bool
28+
}
29+
30+
func (m *mockDockerStatusController) GetCurrentConfig() any {
31+
return &config.Config{APIKey: m.apiKey}
32+
}
33+
34+
func (m *mockDockerStatusController) IsDockerAvailable() bool { return m.dockerAvailable }
35+
36+
func (m *mockDockerStatusController) GetDockerRecoveryStatus() *storage.DockerRecoveryState {
37+
// Mirror the production short-circuit: when recovery is disabled the manager
38+
// returns a synthetic DockerAvailable:true. The endpoint must NOT forward
39+
// this as genuine availability.
40+
return &storage.DockerRecoveryState{DockerAvailable: true}
41+
}
42+
43+
func (m *mockDockerStatusController) GetConfig() (*config.Config, error) {
44+
return &config.Config{
45+
APIKey: m.apiKey,
46+
DockerIsolation: &config.DockerIsolationConfig{Enabled: m.isolationEnabled},
47+
}, nil
48+
}
49+
50+
func TestHandleGetDockerStatus_GenuineAvailability(t *testing.T) {
51+
logger := zap.NewNop().Sugar()
52+
const apiKey = "test-docker-status-key"
53+
54+
tests := []struct {
55+
name string
56+
dockerAvailable bool
57+
isolationEnabled bool
58+
wantAvailable bool
59+
wantIsolationOn bool
60+
}{
61+
{
62+
// The reported bug: no Docker daemon + isolation disabled must NOT
63+
// report docker_available:true (which the synthetic recovery state
64+
// would).
65+
name: "no docker daemon and isolation disabled",
66+
dockerAvailable: false,
67+
isolationEnabled: false,
68+
wantAvailable: false,
69+
wantIsolationOn: false,
70+
},
71+
{
72+
name: "docker present and isolation enabled",
73+
dockerAvailable: true,
74+
isolationEnabled: true,
75+
wantAvailable: true,
76+
wantIsolationOn: true,
77+
},
78+
{
79+
name: "docker present but isolation disabled",
80+
dockerAvailable: true,
81+
isolationEnabled: false,
82+
wantAvailable: true,
83+
wantIsolationOn: false,
84+
},
85+
}
86+
87+
for _, tc := range tests {
88+
t.Run(tc.name, func(t *testing.T) {
89+
ctrl := &mockDockerStatusController{
90+
apiKey: apiKey,
91+
dockerAvailable: tc.dockerAvailable,
92+
isolationEnabled: tc.isolationEnabled,
93+
}
94+
srv := NewServer(ctrl, logger, nil)
95+
96+
req := httptest.NewRequest("GET", "/api/v1/docker/status?apikey="+apiKey, nil)
97+
w := httptest.NewRecorder()
98+
srv.ServeHTTP(w, req)
99+
100+
require.Equal(t, http.StatusOK, w.Code, "body: %s", w.Body.String())
101+
102+
var resp struct {
103+
Success bool `json:"success"`
104+
Data struct {
105+
DockerAvailable bool `json:"docker_available"`
106+
IsolationEnabled bool `json:"isolation_enabled"`
107+
} `json:"data"`
108+
}
109+
require.NoError(t, json.NewDecoder(w.Body).Decode(&resp))
110+
require.True(t, resp.Success)
111+
112+
// docker_available must reflect the GENUINE probe, not the synthetic
113+
// DockerAvailable:true returned by GetDockerRecoveryStatus.
114+
assert.Equal(t, tc.wantAvailable, resp.Data.DockerAvailable,
115+
"docker_available should report genuine daemon availability")
116+
assert.Equal(t, tc.wantIsolationOn, resp.Data.IsolationEnabled,
117+
"isolation_enabled should report docker_isolation.enabled")
118+
})
119+
}
120+
}

internal/httpapi/security_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,7 @@ func (m *baseController) ForceReconnectAllServers(reason string) error { r
244244
func (m *baseController) GetDockerRecoveryStatus() *storage.DockerRecoveryState {
245245
return nil
246246
}
247+
func (m *baseController) IsDockerAvailable() bool { return false }
247248
func (m *baseController) QuarantineServer(serverName string, quarantined bool) error {
248249
return nil
249250
}

internal/httpapi/server.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ type ServerController interface {
7070
RestartServer(serverName string) error
7171
ForceReconnectAllServers(reason string) error
7272
GetDockerRecoveryStatus() *storage.DockerRecoveryState
73+
// IsDockerAvailable reports genuine Docker daemon reachability via a real
74+
// probe (not the synthetic recovery-state value returned when isolation is
75+
// off). Used by /api/v1/docker/status — see MCP-2478.
76+
IsDockerAvailable() bool
7377
QuarantineServer(serverName string, quarantined bool) error
7478
GetQuarantinedServers() ([]map[string]interface{}, error)
7579
UnquarantineServer(serverName string) error
@@ -4612,8 +4616,24 @@ func (s *Server) handleGetDockerStatus(w http.ResponseWriter, r *http.Request) {
46124616
return
46134617
}
46144618

4619+
// Report GENUINE daemon availability from a real probe, not the synthetic
4620+
// DockerAvailable:true that GetDockerRecoveryStatus returns when Docker
4621+
// recovery is disabled (isolation off). See MCP-2478: the dashboard bound
4622+
// its "Docker isolation active" badge to docker_available and lit up on
4623+
// hosts that have no Docker daemon at all. The recovery fields below stay
4624+
// purely diagnostic.
4625+
dockerAvailable := s.controller.IsDockerAvailable()
4626+
4627+
// isolation_enabled lets the UI label the badge "active" only when the user
4628+
// actually turned Docker isolation on AND the daemon is reachable.
4629+
isolationEnabled := false
4630+
if cfg, err := s.controller.GetConfig(); err == nil && cfg != nil && cfg.DockerIsolation != nil {
4631+
isolationEnabled = cfg.DockerIsolation.Enabled
4632+
}
4633+
46154634
response := map[string]interface{}{
4616-
"docker_available": status.DockerAvailable,
4635+
"docker_available": dockerAvailable,
4636+
"isolation_enabled": isolationEnabled,
46174637
"recovery_mode": status.RecoveryMode,
46184638
"failure_count": status.FailureCount,
46194639
"attempts_since_up": status.AttemptsSinceUp,

internal/server/server.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,13 @@ func (s *Server) GetDockerRecoveryStatus() *storage.DockerRecoveryState {
308308
return s.runtime.GetDockerRecoveryStatus()
309309
}
310310

311+
// IsDockerAvailable reports whether the host has a reachable Docker daemon via
312+
// a real probe. The /api/v1/docker/status endpoint uses this for genuine
313+
// availability rather than the synthetic recovery-state value (MCP-2478).
314+
func (s *Server) IsDockerAvailable() bool {
315+
return s.runtime.IsDockerAvailable()
316+
}
317+
311318
// StatusChannel returns a channel that receives status updates
312319
func (s *Server) StatusChannel() <-chan interface{} {
313320
return s.statusCh

0 commit comments

Comments
 (0)