From 99bcb9dfba602a29c13af026b3a84d896ad29a27 Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer A (Kimi)" Date: Sun, 7 Jun 2026 08:34:24 +0000 Subject: [PATCH 1/2] fix(restart): block Restart on removed workspaces to prevent resurrection (#306) The manual Restart handler reads workspace status at line 221-223 but never validated it before flipping to 'provisioning' at line 304-306. A removed workspace could be resurrected via manual Restart because: 1. The sync UPDATE at 306 fires before the async runRestartCycle guard 2. The async guard at 810-812 checks AND status NOT IN ('removed', ...) but by then status is already 'provisioning' Add an early guard after the initial SELECT: if status == 'removed', return 404 (same as not-found, preserving information-hiding). This prevents both the DB flip and the downstream provision cycle. Regression test verifies 404 on removed workspace. Closes :306 from Researcher cleanup audit. --- .../internal/handlers/workspace_restart.go | 7 +++++ .../handlers/workspace_restart_test.go | 27 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/workspace-server/internal/handlers/workspace_restart.go b/workspace-server/internal/handlers/workspace_restart.go index e27b44a5e..a20acb54e 100644 --- a/workspace-server/internal/handlers/workspace_restart.go +++ b/workspace-server/internal/handlers/workspace_restart.go @@ -229,6 +229,13 @@ func (h *WorkspaceHandler) Restart(c *gin.Context) { c.JSON(http.StatusInternalServerError, gin.H{"error": "lookup failed"}) return } + // Block restart if workspace is removed — same 404 as not-found so we don't + // leak that the row ever existed, and to prevent resurrecting a removed + // workspace to 'provisioning' before the async runRestartCycle guard fires. + if status == models.StatusRemoved { + c.JSON(http.StatusNotFound, gin.H{"error": "workspace not found"}) + return + } // Block restart if any ancestor is paused — must resume parent first if paused, parentName := isParentPaused(ctx, id); paused { c.JSON(http.StatusConflict, gin.H{"error": "parent workspace \"" + parentName + "\" is paused — resume it first"}) diff --git a/workspace-server/internal/handlers/workspace_restart_test.go b/workspace-server/internal/handlers/workspace_restart_test.go index 2437762ec..4b6f2697d 100644 --- a/workspace-server/internal/handlers/workspace_restart_test.go +++ b/workspace-server/internal/handlers/workspace_restart_test.go @@ -70,6 +70,33 @@ func TestRestartHandler_DBConnectionError(t *testing.T) { } } +func TestRestartHandler_RemovedWorkspaceReturns404(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + broadcaster := newTestBroadcaster() + handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir()) + + mock.ExpectQuery("SELECT status, name, tier, COALESCE"). + WithArgs("ws-removed"). + WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime"}). + AddRow("removed", "Removed Agent", 1, "claude-code")) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-removed"}} + c.Request = httptest.NewRequest("POST", "/workspaces/ws-removed/restart", nil) + + handler.Restart(c) + + if w.Code != http.StatusNotFound { + t.Errorf("expected 404 for removed workspace, got %d: %s", w.Code, w.Body.String()) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + func TestRestartHandler_AncestorPausedBlocksRestart(t *testing.T) { mock := setupTestDB(t) setupTestRedis(t) -- 2.52.0 From 4855ba400ad123157bc78f97ed5e7e084bca8651 Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer A (Kimi)" Date: Sun, 7 Jun 2026 08:47:13 +0000 Subject: [PATCH 2/2] fix(restart): correct type comparison for StatusRemoved guard The prior commit added a removed-workspace guard using status == models.StatusRemoved, but status is a plain string while models.StatusRemoved is a WorkspaceStatus defined type. Go does not allow direct comparison between different defined types even with identical underlying types, so the build failed. Cast the constant to string for the comparison. --- workspace-server/internal/handlers/workspace_restart.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workspace-server/internal/handlers/workspace_restart.go b/workspace-server/internal/handlers/workspace_restart.go index a20acb54e..4006912ed 100644 --- a/workspace-server/internal/handlers/workspace_restart.go +++ b/workspace-server/internal/handlers/workspace_restart.go @@ -232,7 +232,7 @@ func (h *WorkspaceHandler) Restart(c *gin.Context) { // Block restart if workspace is removed — same 404 as not-found so we don't // leak that the row ever existed, and to prevent resurrecting a removed // workspace to 'provisioning' before the async runRestartCycle guard fires. - if status == models.StatusRemoved { + if status == string(models.StatusRemoved) { c.JSON(http.StatusNotFound, gin.H{"error": "workspace not found"}) return } -- 2.52.0