Merge pull request #966 from Molecule-AI/fix/strip-current-task-public-get

fix(security): strip current_task from public GET response (closes #955)

workspace-server/internal/handlers/handlers_test.go

@@ -1032,8 +1032,9 @@ func TestWorkspaceGet_CurrentTask(t *testing.T) {
 	var resp map[string]interface{}
 	json.Unmarshal(w.Body.Bytes(), &resp)
 
-	if resp["current_task"] != "Analyzing document" {
-		t.Errorf("expected current_task 'Analyzing document', got %v", resp["current_task"])
+	// current_task stripped from public GET response (#955)
+	if _, exists := resp["current_task"]; exists {
+		t.Errorf("current_task should be stripped from public GET response")
 	}
 	if resp["active_tasks"] != float64(2) {
 		t.Errorf("expected active_tasks 2, got %v", resp["active_tasks"])

workspace-server/internal/handlers/workspace.go

@@ -436,11 +436,13 @@ func (h *WorkspaceHandler) Get(c *gin.Context) {
 		return
 	}
 
-	// Strip financial fields — GET /workspaces/:id is on the open router.
-	// Any caller with a valid UUID would otherwise read billing data.
-	// The dedicated budget/spend endpoints are AdminAuth-gated. (#611)
+	// Strip sensitive fields — GET /workspaces/:id is on the open router.
+	// Any caller with a valid UUID would otherwise read operational data.
 	delete(ws, "budget_limit")
 	delete(ws, "monthly_spend")
+	delete(ws, "current_task")      // operational surveillance risk (#955)
+	delete(ws, "last_sample_error") // internal error details
+	delete(ws, "workspace_dir")     // host path disclosure
 
 	c.JSON(http.StatusOK, ws)
 }

workspace-server/internal/handlers/workspace_test.go

@@ -58,8 +58,9 @@ func TestWorkspaceGet_Success(t *testing.T) {
 	if resp["runtime"] != "langgraph" {
 		t.Errorf("expected runtime 'langgraph', got %v", resp["runtime"])
 	}
-	if resp["current_task"] != "working" {
-		t.Errorf("expected current_task 'working', got %v", resp["current_task"])
+	// current_task is stripped from public GET response (#955)
+	if _, exists := resp["current_task"]; exists {
+		t.Errorf("current_task should be stripped from public GET response")
 	}
 
 	if err := mock.ExpectationsWereMet(); err != nil {
@@ -902,6 +903,70 @@ func TestWorkspaceGet_FinancialFieldsStripped(t *testing.T) {
 	}
 }
 
+// TestWorkspaceGet_SensitiveFieldsStripped verifies that GET /workspaces/:id
+// does NOT expose current_task, last_sample_error, or workspace_dir. These
+// leak operational surveillance data and host paths to any caller with a
+// valid UUID. (#955)
+func TestWorkspaceGet_SensitiveFieldsStripped(t *testing.T) {
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+	broadcaster := newTestBroadcaster()
+	handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
+
+	columns := []string{
+		"id", "name", "role", "tier", "status", "agent_card", "url",
+		"parent_id", "active_tasks", "last_error_rate", "last_sample_error",
+		"uptime_seconds", "current_task", "runtime", "workspace_dir", "x", "y", "collapsed",
+		"budget_limit", "monthly_spend",
+	}
+	mock.ExpectQuery("SELECT w.id, w.name").
+		WithArgs("cccccccc-0955-0000-0000-000000000000").
+		WillReturnRows(sqlmock.NewRows(columns).
+			AddRow("cccccccc-0955-0000-0000-000000000000", "Surveillance Test", "worker", 1, "online", []byte(`{}`),
+				"http://localhost:9002", nil, 1, 0.0,
+				"panic: internal error at /secret/path.go:42",
+				100,
+				"Analyzing customer PII for the Q4 report",
+				"langgraph",
+				"/home/user/secret-projects/client-work",
+				0.0, 0.0, false,
+				nil, 0))
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: "cccccccc-0955-0000-0000-000000000000"}}
+	c.Request = httptest.NewRequest("GET", "/workspaces/ws-955", nil)
+
+	handler.Get(c)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
+	}
+
+	var resp map[string]interface{}
+	if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("failed to parse response: %v", err)
+	}
+
+	for _, field := range []string{"current_task", "last_sample_error", "workspace_dir"} {
+		if _, present := resp[field]; present {
+			t.Errorf("%s must not appear in public GET response (got %v)", field, resp[field])
+		}
+	}
+
+	// Sanity: discovery fields still present
+	if resp["name"] != "Surveillance Test" {
+		t.Errorf("expected name 'Surveillance Test', got %v", resp["name"])
+	}
+	if resp["active_tasks"] != float64(1) {
+		t.Errorf("expected active_tasks 1, got %v", resp["active_tasks"])
+	}
+
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet sqlmock expectations: %v", err)
+	}
+}
+
 // TestWorkspaceUpdate_BudgetLimitIgnored verifies that including budget_limit
 // in a PATCH /workspaces/:id body does NOT trigger a DB write. The only write
 // path for budget_limit is PATCH /workspaces/:id/budget (AdminAuth-gated).