diff --git a/workspace-server/internal/handlers/instructions_test.go b/workspace-server/internal/handlers/instructions_test.go new file mode 100644 index 00000000..98760a23 --- /dev/null +++ b/workspace-server/internal/handlers/instructions_test.go @@ -0,0 +1,893 @@ +package handlers + +import ( + "bytes" + "database/sql" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/DATA-DOG/go-sqlmock" + "github.com/gin-gonic/gin" +) + +// ─── request helpers ─────────────────────────────────────────────────────────── + +func newPostRequest(path string, body interface{}) (*httptest.ResponseRecorder, *gin.Context) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + raw, _ := json.Marshal(body) + c.Request = httptest.NewRequest(http.MethodPost, path, bytes.NewReader(raw)) + c.Request.Header.Set("Content-Type", "application/json") + return w, c +} + +func newPutRequest(path string, body interface{}) (*httptest.ResponseRecorder, *gin.Context) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + raw, _ := json.Marshal(body) + c.Request = httptest.NewRequest(http.MethodPut, path, bytes.NewReader(raw)) + c.Request.Header.Set("Content-Type", "application/json") + return w, c +} + +func newDeleteRequest(path string) (*httptest.ResponseRecorder, *gin.Context) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest(http.MethodDelete, path, nil) + return w, c +} + +func newGetRequest(path string) (*httptest.ResponseRecorder, *gin.Context) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest(http.MethodGet, path, nil) + return w, c +} + +// ─── mock row helpers ───────────────────────────────────────────────────────── + +// instructionCols matches the SELECT in List/Resolve. +var instructionCols = []string{ + "id", "scope", "scope_target", "title", "content", + "priority", "enabled", "created_at", "updated_at", +} + +// resolveCols matches the SELECT in Resolve (scope, title, content). +var resolveCols = []string{"scope", "title", "content"} + +// ─── List ──────────────────────────────────────────────────────────────────── + +func TestInstructionsList_ByWorkspaceID(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + wsID := "ws-123-abc" + w, c := newGetRequest("/instructions?workspace_id=" + wsID) + c.Request = httptest.NewRequest(http.MethodGet, "/instructions?workspace_id="+wsID, nil) + + rows := sqlmock.NewRows(instructionCols). + AddRow("inst-1", "global", nil, "Be helpful", "Always be helpful.", 10, true, time.Now(), time.Now()). + AddRow("inst-2", "workspace", &wsID, "Use Claude", "Use Claude Code.", 5, true, time.Now(), time.Now()) + mock.ExpectQuery("SELECT id, scope, scope_target, title, content, priority, enabled, created_at, updated_at"). + WithArgs(wsID). + WillReturnRows(rows) + + h.List(c) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + var out []Instruction + if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil { + t.Fatalf("response not valid JSON: %v", err) + } + if len(out) != 2 { + t.Errorf("expected 2 instructions, got %d", len(out)) + } + if out[0].Scope != "global" { + t.Errorf("first row scope: expected global, got %s", out[0].Scope) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +func TestInstructionsList_ByScope(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + w, c := newGetRequest("/instructions?scope=global") + c.Request = httptest.NewRequest(http.MethodGet, "/instructions?scope=global", nil) + + rows := sqlmock.NewRows(instructionCols). + AddRow("inst-g", "global", nil, "Global Rule", "Follow policy.", 10, true, time.Now(), time.Now()) + mock.ExpectQuery("SELECT id, scope, scope_target, title, content, priority, enabled, created_at, updated_at FROM platform_instructions WHERE 1=1"). + WithArgs("global"). + WillReturnRows(rows) + + h.List(c) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + var out []Instruction + if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil { + t.Fatalf("response not valid JSON: %v", err) + } + if len(out) != 1 || out[0].Scope != "global" { + t.Errorf("unexpected response: %v", out) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +func TestInstructionsList_AllNoParams(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + w, c := newGetRequest("/instructions") + + rows := sqlmock.NewRows(instructionCols) + mock.ExpectQuery("SELECT id, scope, scope_target, title, content, priority, enabled, created_at, updated_at FROM platform_instructions WHERE 1=1"). + WillReturnRows(rows) + + h.List(c) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + var out []Instruction + if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil { + t.Fatalf("response not valid JSON: %v", err) + } + // Empty slice, not nil + if out == nil { + t.Error("expected empty slice, got nil") + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +func TestInstructionsList_DBError(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + w, c := newGetRequest("/instructions") + c.Request = httptest.NewRequest(http.MethodGet, "/instructions", nil) + + mock.ExpectQuery("SELECT id, scope, scope_target, title, content, priority, enabled, created_at, updated_at FROM platform_instructions WHERE 1=1"). + WillReturnError(errors.New("connection refused")) + + h.List(c) + + if w.Code != http.StatusInternalServerError { + t.Fatalf("expected 500, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +// ─── Create ─────────────────────────────────────────────────────────────────── + +func TestInstructionsCreate_ValidGlobal(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + w, c := newPostRequest("/instructions", map[string]interface{}{ + "scope": "global", + "title": "Be Helpful", + "content": "Always be helpful to the user.", + "priority": 10, + }) + + mock.ExpectQuery("INSERT INTO platform_instructions"). + WithArgs("global", nil, "Be Helpful", "Always be helpful to the user.", 10). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("new-inst-1")) + + h.Create(c) + + if w.Code != http.StatusCreated { + t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String()) + } + var out map[string]string + if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil { + t.Fatalf("response not valid JSON: %v", err) + } + if out["id"] != "new-inst-1" { + t.Errorf("expected id new-inst-1, got %s", out["id"]) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +func TestInstructionsCreate_ValidWorkspace(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + wsTarget := "ws-xyz-789" + + w, c := newPostRequest("/instructions", map[string]interface{}{ + "scope": "workspace", + "scope_target": wsTarget, + "title": "Use Claude Code", + "content": "Prefer Claude Code for all tasks.", + "priority": 5, + }) + + mock.ExpectQuery("INSERT INTO platform_instructions"). + WithArgs("workspace", &wsTarget, "Use Claude Code", "Prefer Claude Code for all tasks.", 5). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("ws-inst-2")) + + h.Create(c) + + if w.Code != http.StatusCreated { + t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +func TestInstructionsCreate_MissingScope(t *testing.T) { + setupTestDB(t) + h := NewInstructionsHandler() + + w, c := newPostRequest("/instructions", map[string]interface{}{ + "title": "Missing Scope", + "content": "This has no scope.", + }) + + h.Create(c) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestInstructionsCreate_MissingTitle(t *testing.T) { + setupTestDB(t) + h := NewInstructionsHandler() + + w, c := newPostRequest("/instructions", map[string]interface{}{ + "scope": "global", + "content": "Has no title.", + }) + + h.Create(c) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestInstructionsCreate_MissingContent(t *testing.T) { + setupTestDB(t) + h := NewInstructionsHandler() + + w, c := newPostRequest("/instructions", map[string]interface{}{ + "scope": "global", + "title": "Has no content", + }) + + h.Create(c) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestInstructionsCreate_InvalidScope(t *testing.T) { + setupTestDB(t) + h := NewInstructionsHandler() + + w, c := newPostRequest("/instructions", map[string]interface{}{ + "scope": "team", + "title": "Bad Scope", + "content": "Team scope is not supported yet.", + }) + + h.Create(c) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestInstructionsCreate_WorkspaceScopeNoTarget(t *testing.T) { + setupTestDB(t) + h := NewInstructionsHandler() + + w, c := newPostRequest("/instructions", map[string]interface{}{ + "scope": "workspace", + "title": "Missing Target", + "content": "Workspace scope without scope_target.", + }) + + h.Create(c) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestInstructionsCreate_ContentTooLong(t *testing.T) { + setupTestDB(t) + h := NewInstructionsHandler() + + // Build a string longer than maxInstructionContentLen (8192). + longContent := string(make([]byte, maxInstructionContentLen+1)) + + w, c := newPostRequest("/instructions", map[string]interface{}{ + "scope": "global", + "title": "Too Long", + "content": longContent, + }) + + h.Create(c) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestInstructionsCreate_TitleTooLong(t *testing.T) { + setupTestDB(t) + h := NewInstructionsHandler() + + longTitle := string(make([]byte, 201)) + + w, c := newPostRequest("/instructions", map[string]interface{}{ + "scope": "global", + "title": longTitle, + "content": "Short content.", + }) + + h.Create(c) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestInstructionsCreate_DBError(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + w, c := newPostRequest("/instructions", map[string]interface{}{ + "scope": "global", + "title": "DB Error", + "content": "This will fail.", + }) + + mock.ExpectQuery("INSERT INTO platform_instructions"). + WillReturnError(errors.New("connection refused")) + + h.Create(c) + + if w.Code != http.StatusInternalServerError { + t.Fatalf("expected 500, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +// ─── Update ────────────────────────────────────────────────────────────────── + +func TestInstructionsUpdate_ValidPartial(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + instID := "inst-update-1" + newTitle := "Updated Title" + w, c := newPutRequest("/instructions/"+instID, map[string]interface{}{ + "title": newTitle, + }) + c.Params = []gin.Param{{Key: "id", Value: instID}} + + mock.ExpectExec("UPDATE platform_instructions SET"). + WithArgs(&newTitle, sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), instID). + WillReturnResult(sqlmock.NewResult(0, 1)) + + h.Update(c) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +func TestInstructionsUpdate_AllFields(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + instID := "inst-update-2" + title := "Full Update" + content := "New content body." + priority := 20 + enabled := false + w, c := newPutRequest("/instructions/"+instID, map[string]interface{}{ + "title": title, + "content": content, + "priority": priority, + "enabled": enabled, + }) + c.Params = []gin.Param{{Key: "id", Value: instID}} + + mock.ExpectExec("UPDATE platform_instructions SET"). + WithArgs(&title, &content, &priority, &enabled, instID). + WillReturnResult(sqlmock.NewResult(0, 1)) + + h.Update(c) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +func TestInstructionsUpdate_ContentTooLong(t *testing.T) { + setupTestDB(t) + h := NewInstructionsHandler() + + instID := "inst-too-long" + longContent := string(make([]byte, maxInstructionContentLen+1)) + w, c := newPutRequest("/instructions/"+instID, map[string]interface{}{ + "content": longContent, + }) + c.Params = []gin.Param{{Key: "id", Value: instID}} + + h.Update(c) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestInstructionsUpdate_TitleTooLong(t *testing.T) { + setupTestDB(t) + h := NewInstructionsHandler() + + instID := "inst-title-long" + longTitle := string(make([]byte, 201)) + w, c := newPutRequest("/instructions/"+instID, map[string]interface{}{ + "title": longTitle, + }) + c.Params = []gin.Param{{Key: "id", Value: instID}} + + h.Update(c) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestInstructionsUpdate_NotFound(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + instID := "inst-missing" + w, c := newPutRequest("/instructions/"+instID, map[string]interface{}{ + "title": "New Title", + }) + c.Params = []gin.Param{{Key: "id", Value: instID}} + + mock.ExpectExec("UPDATE platform_instructions SET"). + WillReturnResult(sqlmock.NewResult(0, 0)) + + h.Update(c) + + if w.Code != http.StatusNotFound { + t.Fatalf("expected 404, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +func TestInstructionsUpdate_DBError(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + instID := "inst-db-err" + w, c := newPutRequest("/instructions/"+instID, map[string]interface{}{ + "title": "Error Update", + }) + c.Params = []gin.Param{{Key: "id", Value: instID}} + + mock.ExpectExec("UPDATE platform_instructions SET"). + WillReturnError(errors.New("connection refused")) + + h.Update(c) + + if w.Code != http.StatusInternalServerError { + t.Fatalf("expected 500, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +// ─── Delete ─────────────────────────────────────────────────────────────────── + +func TestInstructionsDelete_Valid(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + instID := "inst-delete-1" + w, c := newDeleteRequest("/instructions/" + instID) + c.Params = []gin.Param{{Key: "id", Value: instID}} + + mock.ExpectExec("DELETE FROM platform_instructions WHERE id = $1"). + WithArgs(instID). + WillReturnResult(sqlmock.NewResult(0, 1)) + + h.Delete(c) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +func TestInstructionsDelete_NotFound(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + instID := "inst-not-there" + w, c := newDeleteRequest("/instructions/" + instID) + c.Params = []gin.Param{{Key: "id", Value: instID}} + + mock.ExpectExec("DELETE FROM platform_instructions WHERE id = $1"). + WithArgs(instID). + WillReturnResult(sqlmock.NewResult(0, 0)) + + h.Delete(c) + + if w.Code != http.StatusNotFound { + t.Fatalf("expected 404, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +func TestInstructionsDelete_DBError(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + instID := "inst-del-err" + w, c := newDeleteRequest("/instructions/" + instID) + c.Params = []gin.Param{{Key: "id", Value: instID}} + + mock.ExpectExec("DELETE FROM platform_instructions WHERE id = $1"). + WillReturnError(errors.New("connection refused")) + + h.Delete(c) + + if w.Code != http.StatusInternalServerError { + t.Fatalf("expected 500, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +// ─── Resolve ────────────────────────────────────────────────────────────────── + +func TestInstructionsResolve_GlobalThenWorkspace(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + wsID := "ws-resolve-1" + w, c := newGetRequest("/workspaces/" + wsID + "/instructions/resolve") + c.Params = []gin.Param{{Key: "id", Value: wsID}} + c.Request = httptest.NewRequest(http.MethodGet, "/workspaces/"+wsID+"/instructions/resolve", nil) + + now := time.Now() + rows := sqlmock.NewRows(resolveCols). + AddRow("global", "Be Helpful", "Always help the user."). + AddRow("global", "Stay on Topic", "Don't diverge."). + AddRow("workspace", "Use Claude Code", "Claude Code is the default runtime.") + mock.ExpectQuery("SELECT scope, title, content FROM platform_instructions"). + WithArgs(wsID). + WillReturnRows(rows) + + h.Resolve(c) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + var out struct { + WorkspaceID string `json:"workspace_id"` + Instructions string `json:"instructions"` + } + if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil { + t.Fatalf("response not valid JSON: %v", err) + } + if out.WorkspaceID != wsID { + t.Errorf("expected workspace_id %s, got %s", wsID, out.WorkspaceID) + } + // Global section must come before workspace section. + if !bytes.Contains([]byte(out.Instructions), []byte("Platform-Wide Rules")) { + t.Error("instructions should contain 'Platform-Wide Rules' section") + } + if !bytes.Contains([]byte(out.Instructions), []byte("Role-Specific Rules")) { + t.Error("instructions should contain 'Role-Specific Rules' section") + } + // Global instructions must appear before workspace instructions. + idxGlobal := bytes.Index([]byte(out.Instructions), []byte("Platform-Wide Rules")) + idxWorkspace := bytes.Index([]byte(out.Instructions), []byte("Role-Specific Rules")) + if idxGlobal >= idxWorkspace { + t.Error("global section should appear before workspace section") + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +func TestInstructionsResolve_EmptyWorkspace(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + wsID := "ws-empty" + w, c := newGetRequest("/workspaces/" + wsID + "/instructions/resolve") + c.Params = []gin.Param{{Key: "id", Value: wsID}} + c.Request = httptest.NewRequest(http.MethodGet, "/workspaces/"+wsID+"/instructions/resolve", nil) + + rows := sqlmock.NewRows(resolveCols) + mock.ExpectQuery("SELECT scope, title, content FROM platform_instructions"). + WithArgs(wsID). + WillReturnRows(rows) + + h.Resolve(c) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + var out struct { + Instructions string `json:"instructions"` + } + if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil { + t.Fatalf("response not valid JSON: %v", err) + } + // No rows → builder writes nothing; empty string returned. + if out.Instructions != "" { + t.Errorf("expected empty instructions for empty workspace, got: %q", out.Instructions) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +func TestInstructionsResolve_DBError(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + wsID := "ws-err" + w, c := newGetRequest("/workspaces/" + wsID + "/instructions/resolve") + c.Params = []gin.Param{{Key: "id", Value: wsID}} + c.Request = httptest.NewRequest(http.MethodGet, "/workspaces/"+wsID+"/instructions/resolve", nil) + + mock.ExpectQuery("SELECT scope, title, content FROM platform_instructions"). + WithArgs(wsID). + WillReturnError(errors.New("connection refused")) + + h.Resolve(c) + + if w.Code != http.StatusInternalServerError { + t.Fatalf("expected 500, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +func TestInstructionsResolve_MissingWorkspaceID(t *testing.T) { + setupTestDB(t) + h := NewInstructionsHandler() + + w, c := newGetRequest("/workspaces//instructions/resolve") + c.Params = []gin.Param{{Key: "id", Value: ""}} + + h.Resolve(c) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } +} + +// ─── scanInstructions edge cases ─────────────────────────────────────────────── + +func TestScanInstructions_ScanError(t *testing.T) { + // A mock rows object that returns a scan error on second row. + badRows := sqlmock.NewRows(instructionCols). + AddRow("inst-ok", "global", nil, "OK", "OK content", 10, true, time.Now(), time.Now()). + RowError(1, errors.New("scan error")). + AddRow("inst-bad", "global", nil, "Bad", "Bad content", 5, true, time.Now(), time.Now()) + + result := scanInstructions(badRows) + // First row should be captured; scan error is logged and skipped. + if len(result) != 1 || result[0].ID != "inst-ok" { + t.Errorf("expected 1 instruction (inst-ok), got: %v", result) + } +} + +// ─── maxInstructionContentLen boundary ──────────────────────────────────────── + +func TestInstructionsCreate_ContentExactlyAtLimit(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + exactContent := string(make([]byte, maxInstructionContentLen)) + w, c := newPostRequest("/instructions", map[string]interface{}{ + "scope": "global", + "title": "At Limit", + "content": exactContent, + }) + + mock.ExpectQuery("INSERT INTO platform_instructions"). + WithArgs("global", nil, "At Limit", exactContent, 0). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("at-limit-1")) + + h.Create(c) + + // Exactly at limit must succeed (8192 chars is acceptable). + if w.Code != http.StatusCreated { + t.Fatalf("expected 201 for content at limit, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +// ─── priority defaults ──────────────────────────────────────────────────────── + +func TestInstructionsCreate_PriorityDefaultsToZero(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + // Body omits priority — expect it defaults to 0. + w, c := newPostRequest("/instructions", map[string]interface{}{ + "scope": "global", + "title": "No Priority", + "content": "Default priority body.", + }) + + mock.ExpectQuery("INSERT INTO platform_instructions"). + WithArgs("global", nil, "No Priority", "Default priority body.", 0). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("no-prio-1")) + + h.Create(c) + + if w.Code != http.StatusCreated { + t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +// ─── nil scope_target for global instructions ───────────────────────────────── + +func TestInstructionsCreate_GlobalScopeNilTarget(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + w, c := newPostRequest("/instructions", map[string]interface{}{ + "scope": "global", + "title": "Global Nil Target", + "content": "Global instruction.", + }) + + // For global scope, scope_target must be SQL NULL. + mock.ExpectQuery("INSERT INTO platform_instructions"). + WithArgs("global", nil, "Global Nil Target", "Global instruction.", 0). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("global-nil-1")) + + h.Create(c) + + if w.Code != http.StatusCreated { + t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +// ─── workspace scope with empty string target (rejected) ───────────────────── + +func TestInstructionsCreate_WorkspaceScopeEmptyStringTarget(t *testing.T) { + setupTestDB(t) + h := NewInstructionsHandler() + + empty := "" + w, c := newPostRequest("/instructions", map[string]interface{}{ + "scope": "workspace", + "scope_target": empty, + "title": "Empty Target", + "content": "Empty workspace target.", + }) + + h.Create(c) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400 for empty string scope_target, got %d: %s", w.Code, w.Body.String()) + } +} + +// ─── Resolve: scope label transitions ──────────────────────────────────────── + +func TestInstructionsResolve_ScopeTransitionOnlyGlobal(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + wsID := "ws-only-global" + w, c := newGetRequest("/workspaces/" + wsID + "/instructions/resolve") + c.Params = []gin.Param{{Key: "id", Value: wsID}} + c.Request = httptest.NewRequest(http.MethodGet, "/workspaces/"+wsID+"/instructions/resolve", nil) + + rows := sqlmock.NewRows(resolveCols). + AddRow("global", "Rule One", "First rule."). + AddRow("global", "Rule Two", "Second rule.") + mock.ExpectQuery("SELECT scope, title, content FROM platform_instructions"). + WithArgs(wsID). + WillReturnRows(rows) + + h.Resolve(c) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + var out struct { + Instructions string `json:"instructions"` + } + if err := json.Unmarshal(w.Body.Bytes(), &out); err != nil { + t.Fatalf("response not valid JSON: %v", err) + } + // Two global instructions share one section header. + if bytes.Count([]byte(out.Instructions), []byte("Platform-Wide Rules")) != 1 { + t.Error("expect exactly one 'Platform-Wide Rules' header for consecutive global rows") + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +// ─── Update: empty body (all nil — no-op update) ───────────────────────────── + +func TestInstructionsUpdate_EmptyBody(t *testing.T) { + mock := setupTestDB(t) + h := NewInstructionsHandler() + + instID := "inst-empty-update" + w, c := newPutRequest("/instructions/"+instID, map[string]interface{}{}) + c.Params = []gin.Param{{Key: "id", Value: instID}} + + // COALESCE(nil, ...) = unchanged; still updates updated_at. + mock.ExpectExec("UPDATE platform_instructions SET"). + WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg(), instID). + WillReturnResult(sqlmock.NewResult(0, 1)) + + h.Update(c) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200 for empty body, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} diff --git a/workspace-server/internal/handlers/org_helpers_loadWorkspaceEnv_test.go b/workspace-server/internal/handlers/org_helpers_loadWorkspaceEnv_test.go new file mode 100644 index 00000000..f7283c71 --- /dev/null +++ b/workspace-server/internal/handlers/org_helpers_loadWorkspaceEnv_test.go @@ -0,0 +1,126 @@ +package handlers + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// setupOrgEnv creates a temp dir with an optional org .env file and returns the dir. +func setupOrgEnv(t *testing.T, orgEnvContent string) string { + t.Helper() + dir := t.TempDir() + if orgEnvContent != "" { + require.NoError(t, os.WriteFile(filepath.Join(dir, ".env"), []byte(orgEnvContent), 0o600)) + } + return dir +} + +func Test_loadWorkspaceEnv_orgRootOnly(t *testing.T) { + org := setupOrgEnv(t, "ORG_VAR=orgval\nORG_DEBUG=true") + vars := loadWorkspaceEnv(org, "") + assert.Equal(t, "orgval", vars["ORG_VAR"]) + assert.Equal(t, "true", vars["ORG_DEBUG"]) +} + +func Test_loadWorkspaceEnv_orgRootMissing(t *testing.T) { + // No .env at org root — should return empty map without error. + dir := t.TempDir() + vars := loadWorkspaceEnv(dir, "") + assertEmpty(t, vars) +} + +func Test_loadWorkspaceEnv_workspaceEnvMerges(t *testing.T) { + org := setupOrgEnv(t, "SHARED=sharedval\nORG_ONLY=orgonly") + wsDir := filepath.Join(org, "myworkspace") + require.NoError(t, os.MkdirAll(wsDir, 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(wsDir, ".env"), []byte("WS_VAR=wsval\nSHARED=overridden"), 0o600)) + + vars := loadWorkspaceEnv(org, "myworkspace") + assert.Equal(t, "wsval", vars["WS_VAR"]) + assert.Equal(t, "overridden", vars["SHARED"]) // workspace overrides org + assert.Equal(t, "orgonly", vars["ORG_ONLY"]) // org vars preserved +} + +func Test_loadWorkspaceEnv_emptyFilesDir(t *testing.T) { + org := setupOrgEnv(t, "VAR=val") + vars := loadWorkspaceEnv(org, "") + assert.Equal(t, "val", vars["VAR"]) +} + +func Test_loadWorkspaceEnv_traversalRejects(t *testing.T) { + // #321 / CWE-22: filesDir "../../../etc" must not escape the org root. + // resolveInsideRoot rejects the traversal so workspace .env is skipped; + // org root .env is still loaded (it's before the guard). + org := setupOrgEnv(t, "INNOCENT=val\nSAFE_WS=wsval") + parent := filepath.Dir(org) + require.NoError(t, os.WriteFile(filepath.Join(parent, ".env"), []byte("MALICIOUS=evil"), 0o600)) + // Also create a workspace dir inside org to prove it IS accessible normally. + wsDir := filepath.Join(org, "legit-workspace") + require.NoError(t, os.MkdirAll(wsDir, 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(wsDir, ".env"), []byte("WS_SECRET=ssh-key-123"), 0o600)) + + // Traversal is blocked. + vars := loadWorkspaceEnv(org, "../../../etc") + // Org root vars present; workspace vars blocked. + assert.Equal(t, "val", vars["INNOCENT"]) + assert.Equal(t, "wsval", vars["SAFE_WS"]) // from org root .env + assert.Empty(t, vars["WS_SECRET"]) // workspace .env blocked by traversal guard + _, hasEvil := vars["MALICIOUS"] + assert.False(t, hasEvil, "MALICIOUS from escaped path must not appear") +} + +func Test_loadWorkspaceEnv_traversalWithDots(t *testing.T) { + // A sibling-traversal attempt: go up one level then into a sibling dir. + // The sibling dir is NOT inside org, so it must be rejected. + org := setupOrgEnv(t, "INNOCENT=val") + parent := filepath.Dir(org) + require.NoError(t, os.MkdirAll(filepath.Join(parent, "sibling"), 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(parent, "sibling/.env"), []byte("LEAKED=secret"), 0o600)) + + vars := loadWorkspaceEnv(org, "../sibling") + // Org vars loaded; sibling vars blocked. + assert.Equal(t, "val", vars["INNOCENT"]) + assert.Empty(t, vars["LEAKED"], "sibling traversal must be rejected") +} + +func Test_loadWorkspaceEnv_absolutePathRejected(t *testing.T) { + // Absolute paths are rejected outright by resolveInsideRoot. + org := setupOrgEnv(t, "INNOCENT=val") + vars := loadWorkspaceEnv(org, "/etc") + assert.Equal(t, "val", vars["INNOCENT"]) // org root still loaded + assert.Empty(t, vars["SAFE_WS"]) +} + +func Test_loadWorkspaceEnv_dotPathRejected(t *testing.T) { + // "." resolves to the org root itself — this is NOT a traversal but + // would create org-root/.env which is the org root .env, not a + // workspace .env. resolveInsideRoot accepts this; the workspace .env + // path is org/.env, which IS the org root .env (already loaded). + // So the correct result is the org vars (same as org root, no change). + org := setupOrgEnv(t, "INNOCENT=val") + vars := loadWorkspaceEnv(org, ".") + // "." passes resolveInsideRoot (resolves to org root, which is valid). + // But workspace path org/.env is the same as org/.env already loaded. + assert.Equal(t, "val", vars["INNOCENT"]) +} + +func Test_loadWorkspaceEnv_emptyOrgRootReturnsEmpty(t *testing.T) { + vars := loadWorkspaceEnv("", "some/dir") + assertEmpty(t, vars) +} + +func Test_loadWorkspaceEnv_missingWorkspaceDir(t *testing.T) { + org := setupOrgEnv(t, "ORG=val") + // Workspace dir doesn't exist — org vars still loaded. + vars := loadWorkspaceEnv(org, "nonexistent") + assert.Equal(t, "val", vars["ORG"]) +} + +func assertEmpty(t *testing.T, m map[string]string) { + t.Helper() + assert.Equal(t, 0, len(m), "expected empty map, got %v", m) +}