diff --git a/docs/feature/rest-error-empty-results.md b/docs/feature/rest-error-empty-results.md index dbc650c0..aea7c924 100644 --- a/docs/feature/rest-error-empty-results.md +++ b/docs/feature/rest-error-empty-results.md @@ -20,3 +20,11 @@ ## F3:实施计划 改动 c2.go 的 DeleteProfile、webshell.go 的 DeleteConnection、database/vulnerability.go 的列表初始化、batch_task_manager.go 的 ListQueuesForAccess 初始化。添加独立 HTTP 合同测试,临时数据库验证缺失、成功删除和重复删除、真实 SQL 失败、空库、筛选无匹配及有结果列表;批量列表含内存路径。运行针对性测试及 handler/database 包测试、git diff --check。更新 changelog 与验证记录。回退使用本分支修复提交的 revert。 + +## F4:实施结果 + +- C2 Profile 删除将 sql.ErrNoRows 映射为 404;真实错误写内部日志,对外通用 500 文案。 +- WebShell 删除使用 errors.Is 识别缺失,对外数据库错误改为通用 500 文案,底层已有日志保留。 +- 漏洞数据库列表、批量队列业务列表初始化为空切片,成功的无记录查询序列化为 [];后者同时覆盖数据库与内存路径。 +- 独立 httptest 回归覆盖缺失、成功删除、重复删除、SQL 错误脱敏、两类空列表、非空结果和批量内存分支。 +- 修复前回归按预期失败,明确捕获原行为;修复后 `go test ./internal/handler -run '^TestREST(Delete|VulnerabilityList|BatchList)Semantics$' -count=1` 通过。 diff --git a/internal/database/vulnerability.go b/internal/database/vulnerability.go index b0a33da2..8c7e827f 100644 --- a/internal/database/vulnerability.go +++ b/internal/database/vulnerability.go @@ -258,7 +258,7 @@ func (db *DB) ListVulnerabilitiesForAccess(limit, offset int, filter Vulnerabili } defer rows.Close() - var vulnerabilities []*Vulnerability + vulnerabilities := make([]*Vulnerability, 0) for rows.Next() { var vuln Vulnerability err := rows.Scan( diff --git a/internal/handler/batch_task_manager.go b/internal/handler/batch_task_manager.go index 7343dec7..e9b17007 100644 --- a/internal/handler/batch_task_manager.go +++ b/internal/handler/batch_task_manager.go @@ -442,7 +442,7 @@ func (m *BatchTaskManager) ListQueues(limit, offset int, status, keyword string) } func (m *BatchTaskManager) ListQueuesForAccess(limit, offset int, status, keyword, userID, scope string) ([]*BatchTaskQueue, int, error) { - var queues []*BatchTaskQueue + queues := make([]*BatchTaskQueue, 0) var total int // 如果数据库可用,从数据库查询 diff --git a/internal/handler/c2.go b/internal/handler/c2.go index c20f3343..af228eaf 100644 --- a/internal/handler/c2.go +++ b/internal/handler/c2.go @@ -1092,7 +1092,12 @@ func (h *C2Handler) UpdateProfile(c *gin.Context) { func (h *C2Handler) DeleteProfile(c *gin.Context) { id := c.Param("id") if err := h.mgr().DB().DeleteC2Profile(id); err != nil { - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + if errors.Is(err, sql.ErrNoRows) { + c.JSON(http.StatusNotFound, gin.H{"error": "profile not found"}) + return + } + h.logger.Error("删除 C2 Profile 失败", zap.Error(err), zap.String("id", id)) + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to delete profile"}) return } c.JSON(http.StatusOK, gin.H{"deleted": true}) diff --git a/internal/handler/rest_semantics_test.go b/internal/handler/rest_semantics_test.go new file mode 100644 index 00000000..a92801ac --- /dev/null +++ b/internal/handler/rest_semantics_test.go @@ -0,0 +1,153 @@ +package handler + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "path/filepath" + "testing" + "time" + + "cyberstrike-ai/internal/c2" + "cyberstrike-ai/internal/database" + "github.com/gin-gonic/gin" + "go.uber.org/zap" +) + +func newRESTSemanticsDB(t *testing.T) *database.DB { + t.Helper() + db, err := database.NewDB(filepath.Join(t.TempDir(), "rest.db"), zap.NewNop()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = db.Close() }) + return db +} + +func restSemanticsRequest(r http.Handler, method, path string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(method, path, nil)) + return w +} + +func TestRESTDeleteSemantics(t *testing.T) { + for _, module := range []string{"profile", "webshell"} { + t.Run(module, func(t *testing.T) { + db := newRESTSemanticsDB(t) + r := gin.New() + table, missing, failure, success := "c2_profiles", "profile not found", "failed to delete profile", `{"deleted":true}` + if module == "profile" { + h := NewC2Handler(c2.NewManager(db, zap.NewNop(), t.TempDir()), zap.NewNop()) + r.DELETE("/objects/:id", h.DeleteProfile) + if err := db.CreateC2Profile(&database.C2Profile{ID: "existing", Name: "test", CreatedAt: time.Now()}); err != nil { + t.Fatal(err) + } + } else { + table, missing, failure, success = "webshell_connections", "connection not found", "failed to delete connection", `{"ok":true}` + h := NewWebShellHandler(zap.NewNop(), db) + r.DELETE("/objects/:id", h.DeleteConnection) + if err := db.CreateWebshellConnection(&database.WebShellConnection{ID: "existing", URL: "https://example.invalid", CreatedAt: time.Now()}); err != nil { + t.Fatal(err) + } + } + for _, tc := range []struct { + id string + code int + body string + }{ + {"missing", http.StatusNotFound, `{"error":"` + missing + `"}`}, + {"existing", http.StatusOK, success}, + {"existing", http.StatusNotFound, `{"error":"` + missing + `"}`}, + } { + w := restSemanticsRequest(r, http.MethodDelete, "/objects/"+tc.id) + if w.Code != tc.code || w.Body.String() != tc.body { + t.Fatalf("delete %s: %d %s, want %d %s", tc.id, w.Code, w.Body.String(), tc.code, tc.body) + } + } + // Break only the temporary database: genuine SQL failures must remain 500, + // with no table name or driver error in the HTTP response. + if _, err := db.Exec("DROP TABLE " + table); err != nil { + t.Fatal(err) + } + w := restSemanticsRequest(r, http.MethodDelete, "/objects/missing") + if w.Code != http.StatusInternalServerError || w.Body.String() != `{"error":"`+failure+`"}` { + t.Fatalf("SQL failure: %d %s", w.Code, w.Body.String()) + } + }) + } +} + +func assertRESTList(t *testing.T, r http.Handler, path, field string, count int) { + t.Helper() + w := restSemanticsRequest(r, http.MethodGet, path) + if w.Code != http.StatusOK { + t.Fatalf("%s: %d %s", path, w.Code, w.Body.String()) + } + var body map[string]json.RawMessage + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { + t.Fatal(err) + } + var items []json.RawMessage + if err := json.Unmarshal(body[field], &items); err != nil || items == nil || len(items) != count { + t.Fatalf("%s: %s=%s, want array of length %d (err=%v)", path, field, body[field], count, err) + } + var total int + if err := json.Unmarshal(body["total"], &total); err != nil || total != count { + t.Fatalf("%s: total=%s, want %d", path, body["total"], count) + } +} + +func TestRESTVulnerabilityListSemantics(t *testing.T) { + db := newRESTSemanticsDB(t) + h := NewVulnerabilityHandler(db, zap.NewNop()) + r := gin.New() + r.GET("/api/vulnerabilities", h.ListVulnerabilities) + assertRESTList(t, r, "/api/vulnerabilities", "vulnerabilities", 0) + if _, err := db.CreateVulnerability(&database.Vulnerability{Title: "test finding", Status: "open", Severity: "low"}); err != nil { + t.Fatal(err) + } + assertRESTList(t, r, "/api/vulnerabilities?status=__none__", "vulnerabilities", 0) + assertRESTList(t, r, "/api/vulnerabilities?status=open", "vulnerabilities", 1) + if _, err := db.Exec("DROP TABLE vulnerabilities"); err != nil { + t.Fatal(err) + } + w := restSemanticsRequest(r, http.MethodGet, "/api/vulnerabilities") + if w.Code != http.StatusInternalServerError { + t.Fatalf("SQL failure became empty success: %d %s", w.Code, w.Body.String()) + } +} + +func TestRESTBatchListSemantics(t *testing.T) { + for _, withDB := range []bool{true, false} { + name := "memory" + if withDB { + name = "database" + } + t.Run(name, func(t *testing.T) { + m := NewBatchTaskManager(zap.NewNop()) + var db *database.DB + if withDB { + db = newRESTSemanticsDB(t) + m.SetDB(db) + } + h := &AgentHandler{batchTaskManager: m, logger: zap.NewNop()} + r := gin.New() + r.GET("/api/batch-tasks", h.ListBatchQueues) + assertRESTList(t, r, "/api/batch-tasks", "queues", 0) + if _, err := m.CreateBatchQueue("test queue", "", "eino_single", "manual", "", "", nil, 1, []string{"test"}); err != nil { + t.Fatal(err) + } + assertRESTList(t, r, "/api/batch-tasks?status=__none__", "queues", 0) + assertRESTList(t, r, "/api/batch-tasks", "queues", 1) + if withDB { + if _, err := db.Exec("DROP TABLE batch_task_queues"); err != nil { + t.Fatal(err) + } + w := restSemanticsRequest(r, http.MethodGet, "/api/batch-tasks") + if w.Code != http.StatusInternalServerError { + t.Fatalf("SQL failure became empty success: %d %s", w.Code, w.Body.String()) + } + } + }) + } +} diff --git a/internal/handler/webshell.go b/internal/handler/webshell.go index 61711174..c23e7404 100644 --- a/internal/handler/webshell.go +++ b/internal/handler/webshell.go @@ -6,6 +6,7 @@ import ( "database/sql" "encoding/base64" "encoding/json" + "errors" "io" "net/http" "net/url" @@ -552,11 +553,11 @@ func (h *WebShellHandler) DeleteConnection(c *gin.Context) { return } if err := h.db.DeleteWebshellConnection(id); err != nil { - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { c.JSON(http.StatusNotFound, gin.H{"error": "connection not found"}) return } - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to delete connection"}) return } if h.audit != nil {