molecule-ai/molecule-core
35
0
Fork 2
Code Issues 16 Pull Requests 2 Actions 49 Packages Projects Releases Wiki Activity

Merge pull request #701 from Molecule-AI/fix/issue-685-686-687-688-input-validation

Browse Source 
fix(security): input validation, route auth, UUID safety (#685 #686 #687 #688)
This commit is contained in:
molecule-ai[bot] 2026-04-17 12:32:03 +00:00 committed by GitHub
commit 63212130e3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 333 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
platform/internal/handlers/handlers_additional_test.go
View File

@ -122,16 +122,16 @@ func TestWorkspaceUpdate_ParentID(t *testing.T) {
// #125 guard: handler now verifies the workspace exists before applying
// the UPDATE. Each PATCH test must mock the EXISTS probe first.
mock.ExpectQuery("SELECT EXISTS.*workspaces WHERE id").
WithArgs("ws-child").
WithArgs("dddddddd-0001-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
mock.ExpectExec("UPDATE workspaces SET parent_id").
WithArgs("ws-child", "ws-parent").
WithArgs("dddddddd-0001-0000-0000-000000000000", "dddddddd-0002-0000-0000-000000000000").
WillReturnResult(sqlmock.NewResult(0, 1))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-child"}}
body := `{"parent_id":"ws-parent"}`
c.Params = gin.Params{{Key: "id", Value: "dddddddd-0001-0000-0000-000000000000"}}
body := `{"parent_id":"dddddddd-0002-0000-0000-000000000000"}`
c.Request = httptest.NewRequest("PATCH", "/workspaces/ws-child", bytes.NewBufferString(body))
c.Request.Header.Set("Content-Type", "application/json")
@ -154,15 +154,15 @@ func TestWorkspaceUpdate_NameOnly(t *testing.T) {
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
mock.ExpectQuery("SELECT EXISTS.*workspaces WHERE id").
WithArgs("ws-rename").
WithArgs("dddddddd-0003-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
mock.ExpectExec("UPDATE workspaces SET name").
WithArgs("ws-rename", "New Name").
WithArgs("dddddddd-0003-0000-0000-000000000000", "New Name").
WillReturnResult(sqlmock.NewResult(0, 1))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-rename"}}
c.Params = gin.Params{{Key: "id", Value: "dddddddd-0003-0000-0000-000000000000"}}
body := `{"name":"New Name"}`
c.Request = httptest.NewRequest("PATCH", "/workspaces/ws-rename", bytes.NewBufferString(body))
c.Request.Header.Set("Content-Type", "application/json")
@ -604,15 +604,15 @@ func TestCheckAccess_ParentChildAllowed(t *testing.T) {
handler := NewDiscoveryHandler()
mock.ExpectQuery("SELECT id, parent_id FROM workspaces WHERE id =").
WithArgs("ws-parent").
WillReturnRows(sqlmock.NewRows([]string{"id", "parent_id"}).AddRow("ws-parent", nil))
WithArgs("dddddddd-0002-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows([]string{"id", "parent_id"}).AddRow("dddddddd-0002-0000-0000-000000000000", nil))
mock.ExpectQuery("SELECT id, parent_id FROM workspaces WHERE id =").
WithArgs("ws-kid").
WillReturnRows(sqlmock.NewRows([]string{"id", "parent_id"}).AddRow("ws-kid", "ws-parent"))
WillReturnRows(sqlmock.NewRows([]string{"id", "parent_id"}).AddRow("ws-kid", "dddddddd-0002-0000-0000-000000000000"))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
body := `{"caller_id":"ws-parent","target_id":"ws-kid"}`
body := `{"caller_id":"dddddddd-0002-0000-0000-000000000000","target_id":"ws-kid"}`
c.Request = httptest.NewRequest("POST", "/registry/check-access", bytes.NewBufferString(body))
c.Request.Header.Set("Content-Type", "application/json")
@ -826,23 +826,23 @@ func TestRestart_ParentPaused(t *testing.T) {
// Workspace lookup succeeds
mock.ExpectQuery("SELECT status, name, tier").
WithArgs("ws-child").
WithArgs("dddddddd-0001-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime"}).
AddRow("offline", "Child Agent", 1, "langgraph"))
// isParentPaused: get parent_id
mock.ExpectQuery("SELECT parent_id FROM workspaces WHERE id").
WithArgs("ws-child").
WillReturnRows(sqlmock.NewRows([]string{"parent_id"}).AddRow("ws-parent"))
WithArgs("dddddddd-0001-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows([]string{"parent_id"}).AddRow("dddddddd-0002-0000-0000-000000000000"))
// isParentPaused: check parent status
mock.ExpectQuery("SELECT status, name FROM workspaces WHERE id").
WithArgs("ws-parent").
WithArgs("dddddddd-0002-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows([]string{"status", "name"}).AddRow("paused", "Parent Agent"))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-child"}}
c.Params = gin.Params{{Key: "id", Value: "dddddddd-0001-0000-0000-000000000000"}}
c.Request = httptest.NewRequest("POST", "/workspaces/ws-child/restart", nil)
handler.Restart(c)

162
platform/internal/handlers/handlers_extended_test.go
View File

@ -15,6 +15,7 @@ import (
// ---------- TestWorkspaceDelete (Extended) ----------
func TestExtended_WorkspaceDelete(t *testing.T) {
const wsDelID = "aaaaaaaa-0000-0000-0000-000000000001"
mock := setupTestDB(t)
setupTestRedis(t)
broadcaster := newTestBroadcaster()
@ -22,7 +23,7 @@ func TestExtended_WorkspaceDelete(t *testing.T) {
// Expect children query — no children
mock.ExpectQuery("SELECT id, name FROM workspaces WHERE parent_id").
WithArgs("ws-del").
WithArgs(wsDelID).
WillReturnRows(sqlmock.NewRows([]string{"id", "name"}))
// #73: batch UPDATE happens BEFORE any container teardown.
@ -40,8 +41,8 @@ func TestExtended_WorkspaceDelete(t *testing.T) {
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-del"}}
c.Request = httptest.NewRequest("DELETE", "/workspaces/ws-del?confirm=true", nil)
c.Params = gin.Params{{Key: "id", Value: wsDelID}}
c.Request = httptest.NewRequest("DELETE", "/workspaces/"+wsDelID+"?confirm=true", nil)
handler.Delete(c)
@ -68,6 +69,7 @@ func TestExtended_WorkspaceDelete(t *testing.T) {
// ---------- TestWorkspaceUpdate (Extended) ----------
func TestExtended_WorkspaceUpdate(t *testing.T) {
const wsUpdID = "aaaaaaaa-0000-0000-0000-000000000002"
mock := setupTestDB(t)
setupTestRedis(t)
broadcaster := newTestBroadcaster()
@ -75,25 +77,25 @@ func TestExtended_WorkspaceUpdate(t *testing.T) {
// #120 fix: existence check runs first — workspace must be found before updates proceed.
mock.ExpectQuery("SELECT EXISTS").
WithArgs("ws-upd").
WithArgs(wsUpdID).
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
// Expect name update
mock.ExpectExec("UPDATE workspaces SET name").
WithArgs("ws-upd", "New Name").
WithArgs(wsUpdID, "New Name").
WillReturnResult(sqlmock.NewResult(0, 1))
// Expect canvas position upsert (x and y both provided)
mock.ExpectExec("INSERT INTO canvas_layouts").
WithArgs("ws-upd", float64(150), float64(250)).
WithArgs(wsUpdID, float64(150), float64(250)).
WillReturnResult(sqlmock.NewResult(0, 1))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-upd"}}
c.Params = gin.Params{{Key: "id", Value: wsUpdID}}
body := `{"name":"New Name","x":150,"y":250}`
c.Request = httptest.NewRequest("PATCH", "/workspaces/ws-upd", bytes.NewBufferString(body))
c.Request = httptest.NewRequest("PATCH", "/workspaces/"+wsUpdID, bytes.NewBufferString(body))
c.Request.Header.Set("Content-Type", "application/json")
handler.Update(c)
@ -638,3 +640,147 @@ func TestExtended_ConfigPatch(t *testing.T) {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
// ─── #687 UUID validation ──────────────────────────────────────────────────
func TestGet_InvalidUUID_Returns400(t *testing.T) {
setupTestDB(t)
setupTestRedis(t)
handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", "/tmp/configs")
for _, badID := range []string{"not-a-uuid", "ws-123", "../etc/passwd", "123"} {
t.Run(badID, func(t *testing.T) {
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: badID}}
c.Request = httptest.NewRequest("GET", "/workspaces/"+badID, nil)
handler.Get(c)
if w.Code != http.StatusBadRequest {
t.Errorf("Get(%q): want 400, got %d", badID, w.Code)
}
})
}
}
func TestUpdate_InvalidUUID_Returns400(t *testing.T) {
setupTestDB(t)
setupTestRedis(t)
handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", "/tmp/configs")
for _, badID := range []string{"not-a-uuid", "ws-upd", "../../secret"} {
t.Run(badID, func(t *testing.T) {
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: badID}}
body := `{"name":"x"}`
c.Request = httptest.NewRequest("PATCH", "/workspaces/"+badID, bytes.NewBufferString(body))
c.Request.Header.Set("Content-Type", "application/json")
handler.Update(c)
if w.Code != http.StatusBadRequest {
t.Errorf("Update(%q): want 400, got %d", badID, w.Code)
}
})
}
}
func TestDelete_InvalidUUID_Returns400(t *testing.T) {
setupTestDB(t)
setupTestRedis(t)
handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", "/tmp/configs")
for _, badID := range []string{"not-a-uuid", "ws-del", "foobar"} {
t.Run(badID, func(t *testing.T) {
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: badID}}
c.Request = httptest.NewRequest("DELETE", "/workspaces/"+badID+"?confirm=true", nil)
handler.Delete(c)
if w.Code != http.StatusBadRequest {
t.Errorf("Delete(%q): want 400, got %d", badID, w.Code)
}
})
}
}
// ─── #685/#688 field validation ───────────────────────────────────────────
func TestValidateWorkspaceFields_Lengths(t *testing.T) {
long256 := string(make([]byte, 256))
long1001 := string(make([]byte, 1001))
long101 := string(make([]byte, 101))
cases := []struct {
label string
name, role, model, runtime string
wantErr bool
}{
{"ok", "ok", "ok role", "gpt-4", "langgraph", false},
{"name_too_long", long256, "", "", "", true},
{"role_too_long", "", long1001, "", "", true},
{"model_too_long", "", "", long101, "", true},
{"runtime_too_long", "", "", "", long101, true},
{"name_newline", "bad\nname", "", "", "", true},
{"role_cr", "", "bad\rrole", "", "", true},
{"model_newline", "", "", "bad\nmodel", "", true},
{"runtime_newline", "", "", "", "bad\nruntime", true},
}
for _, tc := range cases {
t.Run(tc.label, func(t *testing.T) {
err := validateWorkspaceFields(tc.name, tc.role, tc.model, tc.runtime)
if tc.wantErr && err == nil {
t.Errorf("want error, got nil")
}
if !tc.wantErr && err != nil {
t.Errorf("want nil, got %v", err)
}
})
}
}
func TestCreate_FieldValidation_Returns400(t *testing.T) {
setupTestDB(t)
setupTestRedis(t)
handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", "/tmp/configs")
cases := []struct{ label, body string }{
{"name_newline", `{"name":"bad\nname"}`},
{"role_cr", `{"name":"ok","role":"bad\rrole"}`},
}
for _, tc := range cases {
t.Run(tc.label, func(t *testing.T) {
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Request = httptest.NewRequest("POST", "/workspaces", bytes.NewBufferString(tc.body))
c.Request.Header.Set("Content-Type", "application/json")
handler.Create(c)
if w.Code != http.StatusBadRequest {
t.Errorf("Create(%s): want 400, got %d: %s", tc.label, w.Code, w.Body.String())
}
})
}
}
func TestUpdate_FieldValidation_Returns400(t *testing.T) {
setupTestDB(t)
setupTestRedis(t)
handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", "/tmp/configs")
validID := "bbbbbbbb-0000-0000-0000-000000000001"
cases := []struct{ label, body string }{
{"name_newline", `{"name":"bad\nname"}`},
{"role_cr", `{"name":"ok","role":"bad\rrole"}`},
}
for _, tc := range cases {
t.Run(tc.label, func(t *testing.T) {
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: validID}}
c.Request = httptest.NewRequest("PATCH", "/workspaces/"+validID, bytes.NewBufferString(tc.body))
c.Request.Header.Set("Content-Type", "application/json")
handler.Update(c)
if w.Code != http.StatusBadRequest {
t.Errorf("Update(%s): want 400, got %d: %s", tc.label, w.Code, w.Body.String())
}
})
}
}

6
platform/internal/handlers/handlers_test.go
View File

@ -1011,16 +1011,16 @@ func TestWorkspaceGet_CurrentTask(t *testing.T) {
"budget_limit", "monthly_spend",
}
mock.ExpectQuery("SELECT w.id, w.name").
WithArgs("ws-task").
WithArgs("dddddddd-0004-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows(columns).AddRow(
"ws-task", "Task Worker", "worker", 1, "online", []byte("null"), "http://localhost:9000",
"dddddddd-0004-0000-0000-000000000000", "Task Worker", "worker", 1, "online", []byte("null"), "http://localhost:9000",
nil, 2, 0.0, "", 300, "Analyzing document", "langgraph", "", 10.0, 20.0, false,
nil, int64(0),
))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-task"}}
c.Params = gin.Params{{Key: "id", Value: "dddddddd-0004-0000-0000-000000000000"}}
c.Request = httptest.NewRequest("GET", "/workspaces/ws-task", nil)
handler.Get(c)

98
platform/internal/handlers/workspace.go
View File

@ -75,6 +75,13 @@ func (h *WorkspaceHandler) Create(c *gin.Context) {
return
}
// #685/#688: validate field lengths and reject injection characters before
// any DB or provisioner interaction.
if err := validateWorkspaceFields(payload.Name, payload.Role, payload.Model, payload.Runtime); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return
}
id := uuid.New().String()
awarenessNamespace := workspaceAwarenessNamespace(id)
if payload.Tier == 0 {
@ -393,6 +400,12 @@ func (h *WorkspaceHandler) List(c *gin.Context) {
func (h *WorkspaceHandler) Get(c *gin.Context) {
id := c.Param("id")
// #687: reject non-UUID IDs before hitting the DB.
if err := validateWorkspaceID(id); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return
}
row := db.DB.QueryRowContext(c.Request.Context(), `
SELECT w.id, w.name, COALESCE(w.role, ''), w.tier, w.status,
COALESCE(w.agent_card, 'null'::jsonb), COALESCE(w.url, ''),
@ -531,12 +544,34 @@ var sensitiveUpdateFields = map[string]struct{}{
func (h *WorkspaceHandler) Update(c *gin.Context) {
id := c.Param("id")
// #687: reject non-UUID IDs before hitting the DB.
if err := validateWorkspaceID(id); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return
}
var body map[string]interface{}
if err := c.ShouldBindJSON(&body); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return
}
// #685/#688: validate string fields for length and injection safety.
strField := func(key string) string {
if v, ok := body[key]; ok {
if s, ok := v.(string); ok {
return s
}
}
return ""
}
if err := validateWorkspaceFields(
strField("name"), strField("role"), "" /*model not patchable*/, strField("runtime"),
); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return
}
ctx := c.Request.Context()
// Auth is fully enforced at the router layer (WorkspaceAuth middleware, #680).
@ -647,6 +682,12 @@ func (h *WorkspaceHandler) Delete(c *gin.Context) {
ctx := c.Request.Context()
confirm := c.Query("confirm") == "true"
// #687: reject non-UUID IDs before hitting the DB.
if err := validateWorkspaceID(id); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return
}
// Check for children
rows, err := db.DB.QueryContext(ctx,
`SELECT id, name FROM workspaces WHERE parent_id = $1 AND status != 'removed'`, id)
@ -773,3 +814,60 @@ func (h *WorkspaceHandler) Delete(c *gin.Context) {
c.JSON(http.StatusOK, gin.H{"status": "removed", "cascade_deleted": len(descendantIDs)})
}
// validateWorkspaceID returns an error when id is not a valid UUID.
// #687: prevents 500s from Postgres when a garbage string (e.g. ../../etc/passwd)
// is passed as the :id path parameter.
func validateWorkspaceID(id string) error {
if _, err := uuid.Parse(id); err != nil {
return fmt.Errorf("invalid workspace id")
}
return nil
}
// yamlSpecialChars is the set of YAML-special characters banned from workspace
// name and role. Newlines are handled separately below (same error message for
// all four fields); these additional characters target YAML block indicators,
// flow-sequence/mapping delimiters, and shell-expansion metacharacters that
// yamlQuote does NOT escape inside a double-quoted scalar (#685).
const yamlSpecialChars = "{}[]|>*&!"
// validateWorkspaceFields enforces maximum field lengths and rejects characters
// that could enable YAML-injection in downstream provisioning paths.
// #685 (defence-in-depth over yamlQuote — newline + YAML-special chars in name/role),
// #688 (max field lengths).
func validateWorkspaceFields(name, role, model, runtime string) error {
// All four fields: reject newline / carriage-return.
for _, f := range []struct{ label, val string }{
{"name", name},
{"role", role},
{"model", model},
{"runtime", runtime},
} {
if strings.ContainsAny(f.val, "\n\r") {
return fmt.Errorf("%s must not contain newline characters", f.label)
}
}
// name and role only: reject YAML-special characters (#685).
for _, f := range []struct{ label, val string }{
{"name", name},
{"role", role},
} {
if strings.ContainsAny(f.val, yamlSpecialChars) {
return fmt.Errorf("%s contains invalid characters", f.label)
}
}
if len(name) > 255 {
return fmt.Errorf("name must be at most 255 characters")
}
if len(role) > 1000 {
return fmt.Errorf("role must be at most 1000 characters")
}
if len(model) > 100 {
return fmt.Errorf("model must be at most 100 characters")
}
if len(runtime) > 100 {
return fmt.Errorf("runtime must be at most 100 characters")
}
return nil
}

20
platform/internal/handlers/workspace_budget_test.go
View File

@ -45,9 +45,9 @@ func TestWorkspaceBudget_Get_NilLimit(t *testing.T) {
handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir())
mock.ExpectQuery("SELECT w.id, w.name").
WithArgs("ws-nobudget").
WithArgs("dddddddd-0005-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows(wsColumns).
AddRow("ws-nobudget", "Free Agent", "worker", 1, "online",
AddRow("dddddddd-0005-0000-0000-000000000000", "Free Agent", "worker", 1, "online",
[]byte(`{}`), "http://localhost:9001",
nil, 0, 0.0, "", 0, "", "langgraph", "",
0.0, 0.0, false,
@ -56,7 +56,7 @@ func TestWorkspaceBudget_Get_NilLimit(t *testing.T) {
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-nobudget"}}
c.Params = gin.Params{{Key: "id", Value: "dddddddd-0005-0000-0000-000000000000"}}
c.Request = httptest.NewRequest("GET", "/workspaces/ws-nobudget", nil)
handler.Get(c)
@ -88,9 +88,9 @@ func TestWorkspaceBudget_Get_WithLimit(t *testing.T) {
handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir())
mock.ExpectQuery("SELECT w.id, w.name").
WithArgs("ws-limited").
WithArgs("dddddddd-0006-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows(wsColumns).
AddRow("ws-limited", "Capped Agent", "worker", 1, "online",
AddRow("dddddddd-0006-0000-0000-000000000000", "Capped Agent", "worker", 1, "online",
[]byte(`{}`), "http://localhost:9002",
nil, 0, 0.0, "", 0, "", "langgraph", "",
0.0, 0.0, false,
@ -99,7 +99,7 @@ func TestWorkspaceBudget_Get_WithLimit(t *testing.T) {
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-limited"}}
c.Params = gin.Params{{Key: "id", Value: "dddddddd-0006-0000-0000-000000000000"}}
c.Request = httptest.NewRequest("GET", "/workspaces/ws-limited", nil)
handler.Get(c)
@ -186,13 +186,13 @@ func TestWorkspaceBudget_Update_SetLimit(t *testing.T) {
// Only the existence probe fires; no UPDATE for budget_limit.
mock.ExpectQuery("SELECT EXISTS.*workspaces WHERE id").
WithArgs("ws-upd-budget").
WithArgs("dddddddd-0007-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
// No ExpectExec for budget_limit — sqlmock will fail if one is issued.
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-upd-budget"}}
c.Params = gin.Params{{Key: "id", Value: "dddddddd-0007-0000-0000-000000000000"}}
body := `{"budget_limit":500}`
c.Request = httptest.NewRequest("PATCH", "/workspaces/ws-upd-budget", bytes.NewBufferString(body))
c.Request.Header.Set("Content-Type", "application/json")
@ -216,13 +216,13 @@ func TestWorkspaceBudget_Update_ClearLimit(t *testing.T) {
// Only the existence probe fires; no UPDATE for budget_limit.
mock.ExpectQuery("SELECT EXISTS.*workspaces WHERE id").
WithArgs("ws-clear-budget").
WithArgs("dddddddd-0008-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
// No ExpectExec — a budget_limit write here would re-open the vulnerability.
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-clear-budget"}}
c.Params = gin.Params{{Key: "id", Value: "dddddddd-0008-0000-0000-000000000000"}}
body := `{"budget_limit":null}`
c.Request = httptest.NewRequest("PATCH", "/workspaces/ws-clear-budget", bytes.NewBufferString(body))
c.Request.Header.Set("Content-Type", "application/json")

92
platform/internal/handlers/workspace_test.go
View File

@ -27,16 +27,16 @@ func TestWorkspaceGet_Success(t *testing.T) {
"budget_limit", "monthly_spend",
}
mock.ExpectQuery("SELECT w.id, w.name").
WithArgs("ws-get-1").
WithArgs("cccccccc-0001-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows(columns).
AddRow("ws-get-1", "My Agent", "worker", 1, "online", []byte(`{"name":"test"}`),
AddRow("cccccccc-0001-0000-0000-000000000000", "My Agent", "worker", 1, "online", []byte(`{"name":"test"}`),
"http://localhost:8001", nil, 2, 0.05, "", 3600, "working", "langgraph",
"", 10.0, 20.0, false,
nil, 0))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-get-1"}}
c.Params = gin.Params{{Key: "id", Value: "cccccccc-0001-0000-0000-000000000000"}}
c.Request = httptest.NewRequest("GET", "/workspaces/ws-get-1", nil)
handler.Get(c)
@ -74,12 +74,12 @@ func TestWorkspaceGet_NotFound(t *testing.T) {
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
mock.ExpectQuery("SELECT w.id, w.name").
WithArgs("ws-nonexistent").
WithArgs("cccccccc-0002-0000-0000-000000000000").
WillReturnError(sql.ErrNoRows)
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-nonexistent"}}
c.Params = gin.Params{{Key: "id", Value: "cccccccc-0002-0000-0000-000000000000"}}
c.Request = httptest.NewRequest("GET", "/workspaces/ws-nonexistent", nil)
handler.Get(c)
@ -100,12 +100,12 @@ func TestWorkspaceGet_DBError(t *testing.T) {
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
mock.ExpectQuery("SELECT w.id, w.name").
WithArgs("ws-dberr").
WithArgs("cccccccc-0003-0000-0000-000000000000").
WillReturnError(sql.ErrConnDone)
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-dberr"}}
c.Params = gin.Params{{Key: "id", Value: "cccccccc-0003-0000-0000-000000000000"}}
c.Request = httptest.NewRequest("GET", "/workspaces/ws-dberr", nil)
handler.Get(c)
@ -406,7 +406,7 @@ func TestWorkspaceUpdate_BadJSON(t *testing.T) {
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-upd"}}
c.Params = gin.Params{{Key: "id", Value: "cccccccc-0004-0000-0000-000000000000"}}
c.Request = httptest.NewRequest("PATCH", "/workspaces/ws-upd", bytes.NewBufferString("not json"))
c.Request.Header.Set("Content-Type", "application/json")
@ -425,22 +425,22 @@ func TestWorkspaceUpdate_MultipleFields(t *testing.T) {
// #125: existence probe fires once before any field update.
mock.ExpectQuery("SELECT EXISTS.*workspaces WHERE id").
WithArgs("ws-multi").
WithArgs("cccccccc-0005-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
// Expect name, role, and tier updates
mock.ExpectExec("UPDATE workspaces SET name").
WithArgs("ws-multi", "Updated Agent").
WithArgs("cccccccc-0005-0000-0000-000000000000", "Updated Agent").
WillReturnResult(sqlmock.NewResult(0, 1))
mock.ExpectExec("UPDATE workspaces SET role").
WithArgs("ws-multi", "manager").
WithArgs("cccccccc-0005-0000-0000-000000000000", "manager").
WillReturnResult(sqlmock.NewResult(0, 1))
mock.ExpectExec("UPDATE workspaces SET tier").
WithArgs("ws-multi", float64(3)).
WithArgs("cccccccc-0005-0000-0000-000000000000", float64(3)).
WillReturnResult(sqlmock.NewResult(0, 1))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-multi"}}
c.Params = gin.Params{{Key: "id", Value: "cccccccc-0005-0000-0000-000000000000"}}
body := `{"name":"Updated Agent","role":"manager","tier":3}`
c.Request = httptest.NewRequest("PATCH", "/workspaces/ws-multi", bytes.NewBufferString(body))
@ -472,15 +472,15 @@ func TestWorkspaceUpdate_RuntimeField(t *testing.T) {
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
mock.ExpectQuery("SELECT EXISTS.*workspaces WHERE id").
WithArgs("ws-rt").
WithArgs("cccccccc-0006-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
mock.ExpectExec("UPDATE workspaces SET runtime").
WithArgs("ws-rt", "claude-code").
WithArgs("cccccccc-0006-0000-0000-000000000000", "claude-code").
WillReturnResult(sqlmock.NewResult(0, 1))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-rt"}}
c.Params = gin.Params{{Key: "id", Value: "cccccccc-0006-0000-0000-000000000000"}}
body := `{"runtime":"claude-code"}`
c.Request = httptest.NewRequest("PATCH", "/workspaces/ws-rt", bytes.NewBufferString(body))
@ -507,14 +507,14 @@ func TestWorkspaceDelete_ConfirmationRequired(t *testing.T) {
// Children query returns 2 children
mock.ExpectQuery("SELECT id, name FROM workspaces WHERE parent_id").
WithArgs("ws-parent").
WithArgs("cccccccc-0007-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows([]string{"id", "name"}).
AddRow("ws-child-1", "Child One").
AddRow("ws-child-2", "Child Two"))
AddRow("cccccccc-0008-0000-0000-000000000000", "Child One").
AddRow("cccccccc-0009-0000-0000-000000000000", "Child Two"))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-parent"}}
c.Params = gin.Params{{Key: "id", Value: "cccccccc-0007-0000-0000-000000000000"}}
// No ?confirm=true
c.Request = httptest.NewRequest("DELETE", "/workspaces/ws-parent", nil)
@ -552,14 +552,14 @@ func TestWorkspaceDelete_CascadeWithChildren(t *testing.T) {
// Children query returns 1 child
mock.ExpectQuery("SELECT id, name FROM workspaces WHERE parent_id").
WithArgs("ws-parent-del").
WithArgs("cccccccc-000a-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows([]string{"id", "name"}).
AddRow("ws-child-del", "Child Agent"))
AddRow("cccccccc-000b-0000-0000-000000000000", "Child Agent"))
// Descendant CTE query returns the recursive set (1 descendant: ws-child-del)
mock.ExpectQuery("WITH RECURSIVE descendants").
WithArgs("ws-parent-del").
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("ws-child-del"))
WithArgs("cccccccc-000a-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("cccccccc-000b-0000-0000-000000000000"))
// #73: single batch UPDATE covering [self + descendants] BEFORE stopping
// containers (prevents heartbeat/restart resurrection races).
@ -580,7 +580,7 @@ func TestWorkspaceDelete_CascadeWithChildren(t *testing.T) {
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-parent-del"}}
c.Params = gin.Params{{Key: "id", Value: "cccccccc-000a-0000-0000-000000000000"}}
c.Request = httptest.NewRequest("DELETE", "/workspaces/ws-parent-del?confirm=true", nil)
handler.Delete(c)
@ -612,12 +612,12 @@ func TestWorkspaceDelete_ChildrenQueryError(t *testing.T) {
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
mock.ExpectQuery("SELECT id, name FROM workspaces WHERE parent_id").
WithArgs("ws-err-del").
WithArgs("cccccccc-000c-0000-0000-000000000000").
WillReturnError(sql.ErrConnDone)
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-err-del"}}
c.Params = gin.Params{{Key: "id", Value: "cccccccc-000c-0000-0000-000000000000"}}
c.Request = httptest.NewRequest("DELETE", "/workspaces/ws-err-del?confirm=true", nil)
handler.Delete(c)
@ -781,32 +781,30 @@ func TestWorkspaceState_ValidTokenReturnsStatus(t *testing.T) {
// without a bearer token. Sensitive fields (tier/parent_id/runtime/
// workspace_dir) require a valid admin bearer once any live token exists.
// TestWorkspaceUpdate_CosmeticField_Passthrough verifies that a cosmetic-field
// PATCH (name, role, x, y) is processed by the handler without any DB auth query.
// Auth is fully enforced by WorkspaceAuth middleware before the handler runs (#680).
func TestWorkspaceUpdate_CosmeticField_Passthrough(t *testing.T) {
func TestWorkspaceUpdate_CosmeticField_NoBearer_FailOpen_NoTokens(t *testing.T) {
mock := setupTestDB(t)
setupTestRedis(t)
broadcaster := newTestBroadcaster()
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
// Body contains only cosmetic field → no wsauth probe ever fires.
mock.ExpectQuery("SELECT EXISTS.*workspaces WHERE id").
WithArgs("ws-cosmetic").
WithArgs("cccccccc-000d-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
mock.ExpectExec("UPDATE workspaces SET name").
WithArgs("ws-cosmetic", "Cosmetic").
WithArgs("cccccccc-000d-0000-0000-000000000000", "Cosmetic").
WillReturnResult(sqlmock.NewResult(0, 1))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-cosmetic"}}
c.Params = gin.Params{{Key: "id", Value: "cccccccc-000d-0000-0000-000000000000"}}
c.Request = httptest.NewRequest("PATCH", "/workspaces/ws-cosmetic",
bytes.NewBufferString(`{"name":"Cosmetic"}`))
c.Request.Header.Set("Content-Type", "application/json")
handler.Update(c)
if w.Code != http.StatusOK {
t.Errorf("cosmetic PATCH: got %d, want 200: %s", w.Code, w.Body.String())
t.Errorf("cosmetic PATCH (no bearer) should pass; got %d: %s", w.Code, w.Body.String())
}
}
@ -824,16 +822,16 @@ func TestWorkspaceUpdate_SensitiveField_AuthEnforcedByMiddleware(t *testing.T) {
// No workspace_auth_tokens query expected — auth is middleware's responsibility.
mock.ExpectQuery("SELECT EXISTS.*workspaces WHERE id").
WithArgs("ws-owned").
WithArgs("cccccccc-000e-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
mock.ExpectExec("UPDATE workspaces SET tier").
WithArgs("ws-owned", float64(3)).
WithArgs("cccccccc-000e-0000-0000-000000000000", float64(3)).
WillReturnResult(sqlmock.NewResult(0, 1))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-owned"}}
c.Request = httptest.NewRequest("PATCH", "/workspaces/ws-owned",
c.Params = gin.Params{{Key: "id", Value: "cccccccc-000e-0000-0000-000000000000"}}
c.Request = httptest.NewRequest("PATCH", "/workspaces/cccccccc-000e-0000-0000-000000000000",
bytes.NewBufferString(`{"tier":3}`))
c.Request.Header.Set("Content-Type", "application/json")
// WorkspaceAuth middleware would have validated the bearer before this runs.
@ -866,16 +864,16 @@ func TestWorkspaceGet_FinancialFieldsStripped(t *testing.T) {
}
// Populate with non-zero financial values to confirm they are stripped.
mock.ExpectQuery("SELECT w.id, w.name").
WithArgs("ws-fin-1").
WithArgs("cccccccc-0010-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows(columns).
AddRow("ws-fin-1", "Finance Test", "worker", 1, "online", []byte(`{}`),
AddRow("cccccccc-0010-0000-0000-000000000000", "Finance Test", "worker", 1, "online", []byte(`{}`),
"http://localhost:9001", nil, 0, 0.0, "", 0, "", "langgraph",
"", 0.0, 0.0, false,
int64(50000), int64(12500))) // budget_limit=500 USD, spend=125 USD
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-fin-1"}}
c.Params = gin.Params{{Key: "id", Value: "cccccccc-0010-0000-0000-000000000000"}}
c.Request = httptest.NewRequest("GET", "/workspaces/ws-fin-1", nil)
handler.Get(c)
@ -917,16 +915,16 @@ func TestWorkspaceUpdate_BudgetLimitIgnored(t *testing.T) {
// Only the existence probe fires — no UPDATE for budget_limit.
mock.ExpectQuery("SELECT EXISTS.*workspaces WHERE id").
WithArgs("ws-budget-test").
WithArgs("cccccccc-0011-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
// name update is the only expected write
mock.ExpectExec("UPDATE workspaces SET name").
WithArgs("ws-budget-test", "Safe Name").
WithArgs("cccccccc-0011-0000-0000-000000000000", "Safe Name").
WillReturnResult(sqlmock.NewResult(0, 1))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-budget-test"}}
c.Params = gin.Params{{Key: "id", Value: "cccccccc-0011-0000-0000-000000000000"}}
// Send budget_limit alongside an innocuous field.
body := `{"name":"Safe Name","budget_limit":null}`
c.Request = httptest.NewRequest("PATCH", "/workspaces/ws-budget-test",
@ -954,13 +952,13 @@ func TestWorkspaceUpdate_BudgetLimitOnly_Ignored(t *testing.T) {
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
mock.ExpectQuery("SELECT EXISTS.*workspaces WHERE id").
WithArgs("ws-budget-only").
WithArgs("cccccccc-0012-0000-0000-000000000000").
WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true))
// No UPDATE expected — budget_limit must be silently skipped.
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-budget-only"}}
c.Params = gin.Params{{Key: "id", Value: "cccccccc-0012-0000-0000-000000000000"}}
c.Request = httptest.NewRequest("PATCH", "/workspaces/ws-budget-only",
bytes.NewBufferString(`{"budget_limit":999999}`))
c.Request.Header.Set("Content-Type", "application/json")

9
platform/internal/router/router.go
View File

@ -372,11 +372,14 @@ func Setup(hub *ws.Hub, broadcaster *events.Broadcaster, prov *provisioner.Provi
// Templates
tmplh := handlers.NewTemplatesHandler(configsDir, dockerCli)
r.GET("/templates", tmplh.List)
// #686: GET /templates lists all template names+metadata from configsDir.
// Open access lets unauthenticated callers enumerate org configurations and
// installed plugins. AdminAuth-gate it alongside POST /templates/import.
// #190: POST /templates/import writes arbitrary files into configsDir.
// Must be admin-gated — same class as /bundles/import (#164) and /org/import.
{
tmplAdmin := r.Group("", middleware.AdminAuth(db.DB))
tmplAdmin.GET("/templates", tmplh.List)
tmplAdmin.POST("/templates/import", tmplh.Import)
}
wsAuth.GET("/shared-context", tmplh.SharedContext)
@ -429,7 +432,9 @@ func Setup(hub *ws.Hub, broadcaster *events.Broadcaster, prov *provisioner.Provi
// Org Templates
orgDir := findOrgDir(configsDir)
orgh := handlers.NewOrgHandler(wh, broadcaster, prov, channelMgr, configsDir, orgDir)
r.GET("/org/templates", orgh.ListTemplates)
// #686: GET /org/templates exposes the org template catalogue (names, roles,
// configured system prompts). AdminAuth-gate to match /org/import.
r.GET("/org/templates", middleware.AdminAuth(db.DB), orgh.ListTemplates)
// /org/import can create arbitrary workspaces from an uploaded YAML — it
// must be an admin-gated route. The handler also path-sanitizes
// `dir`/`template`/`files_dir` via resolveInsideRoot, but defence-in-