diff --git a/platform/internal/handlers/handlers_extended_test.go b/platform/internal/handlers/handlers_extended_test.go index 68df80ed..1e6f3a53 100644 --- a/platform/internal/handlers/handlers_extended_test.go +++ b/platform/internal/handlers/handlers_extended_test.go @@ -73,6 +73,11 @@ func TestExtended_WorkspaceUpdate(t *testing.T) { broadcaster := newTestBroadcaster() handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", "/tmp/configs") + // #120 fix: existence check runs first — workspace must be found before updates proceed. + mock.ExpectQuery("SELECT EXISTS"). + WithArgs("ws-upd"). + WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true)) + // Expect name update mock.ExpectExec("UPDATE workspaces SET name"). WithArgs("ws-upd", "New Name"). @@ -110,6 +115,48 @@ func TestExtended_WorkspaceUpdate(t *testing.T) { } } +// TestExtended_WorkspaceUpdate_NotFound verifies the #120 fix: PATCH /workspaces/:id +// returns 404 (not 200) when the workspace does not exist in the DB. +// +// Before PR #125, the handler ran blind UPDATEs that matched zero rows and still +// returned {"status":"updated"} HTTP 200 — allowing an attacker to probe and +// speculatively modify workspace attributes (name, tier, parent_id, runtime, +// workspace_dir) without any observable error. The existence guard must fire +// and return 404 before any UPDATE is attempted. +func TestExtended_WorkspaceUpdate_NotFound(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + broadcaster := newTestBroadcaster() + handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", "/tmp/configs") + + // Existence check returns false — workspace does not exist. + mock.ExpectQuery("SELECT EXISTS"). + WithArgs("00000000-0000-0000-0000-000000000000"). + WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false)) + + // No UPDATE or INSERT should follow — the handler must short-circuit at 404. + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "00000000-0000-0000-0000-000000000000"}} + + body := `{"name":"probe"}` + c.Request = httptest.NewRequest("PATCH", + "/workspaces/00000000-0000-0000-0000-000000000000", + bytes.NewBufferString(body)) + c.Request.Header.Set("Content-Type", "application/json") + + handler.Update(c) + + if w.Code != http.StatusNotFound { + t.Errorf("#120 regression: expected 404 for nonexistent workspace, got %d: %s", + w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + // ---------- TestWorkspaceRestart (Extended) ---------- func TestExtended_WorkspaceRestart_NoProvisioner(t *testing.T) { diff --git a/platform/internal/middleware/wsauth_middleware_test.go b/platform/internal/middleware/wsauth_middleware_test.go index 50ba5b91..95b79bbb 100644 --- a/platform/internal/middleware/wsauth_middleware_test.go +++ b/platform/internal/middleware/wsauth_middleware_test.go @@ -472,3 +472,50 @@ func TestAdminAuth_InvalidBearer_Returns401(t *testing.T) { t.Errorf("unmet sqlmock expectations: %v", err) } } + +// ──────────────────────────────────────────────────────────────────────────── +// Issue #120 regression — unauthenticated PATCH /workspaces/:id +// +// Before PR #125, PATCH /workspaces/:id was registered outside the wsAdmin +// group and did NOT enforce AdminAuth. An attacker could change workspace +// name, tier, parent_id, runtime, or workspace_dir without any token. +// Security Auditor confirmed the live exploit: +// curl -X PATCH .../workspaces/00000000-.../ -d '{"name":"probe"}' → 200 +// +// This test asserts AdminAuth applied to the PATCH route blocks unauthenticated +// requests — the route-level fix in router.go is the enforcement point. +// ──────────────────────────────────────────────────────────────────────────── + +// TestAdminAuth_Issue120_PatchWorkspace_NoBearer_Returns401 documents the #120 +// attack vector and verifies that AdminAuth returns 401 for PATCH without a token. +func TestAdminAuth_Issue120_PatchWorkspace_NoBearer_Returns401(t *testing.T) { + mockDB, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer mockDB.Close() + + // HasAnyLiveTokenGlobal returns 1 — at least one workspace is token-enrolled. + mock.ExpectQuery(hasAnyLiveTokenGlobalQuery). + WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1)) + + r := gin.New() + // Mirror the PR #125 router change: PATCH is inside the wsAdmin AdminAuth group. + r.PATCH("/workspaces/:id", AdminAuth(mockDB), func(c *gin.Context) { + c.JSON(http.StatusOK, gin.H{"status": "updated"}) + }) + + w := httptest.NewRecorder() + // #120 attack: no Authorization header on PATCH. + req, _ := http.NewRequest(http.MethodPatch, + "/workspaces/00000000-0000-0000-0000-000000000000", + nil) + r.ServeHTTP(w, req) + + if w.Code != http.StatusUnauthorized { + t.Errorf("#120 PATCH no-bearer: expected 401, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +}