diff --git a/docs/bug-sheet-review.md b/docs/bug-sheet-review.md index 34e823cc..aa86cd10 100644 --- a/docs/bug-sheet-review.md +++ b/docs/bug-sheet-review.md @@ -16,6 +16,9 @@ | --- | --- | --- | | C2 wait/cancel 资源权限 | handler 直接按 ID 查询和取消，缺少 c2ResourceAllowed | 与同模块接口一致，先校验任务访问权限，再执行 | | 32 纯文本批量/机器人任务缺少执行证据 | batch_queue_executor 和 finalizeRobotAgentSuccess 显式传 true | 与普通对话的默认策略一致，允许无工具文本回复，保留 pending/HITL 检查 | +| 30、31 角色名路径字符/长度 | 名称未校验，文件名会转换，存在碰撞 | 拒绝路径和控制字符、首尾空格、超过 64 字符及文件名碰撞 | +| 工作流悬空引用 | 角色保存与工作流删除均未检查引用 | 保存时验证存在；删除引用中的工作流返回冲突 | +| 4 登录用户名为空 | 前端无 required，后端空用户名默认 admin | 登录接口及页面拒绝空白/超过 64 字符用户名 | ## 不适用 @@ -23,4 +26,7 @@ ## 验证 -待填写实际执行结果。 +- `go test ./internal/handler ./internal/app ./internal/agentfinalizer` 通过。 +- 新增 C2 跨用户禁止等待/取消、所有者正常操作、机器人纯文本交付、角色名称边界、工作流引用、登录用户名测试。 +- 前端已有测试存在 HITL 断言失败，正在核实基线；未修改 HITL 脚本。 +- 当前尚未完成全部表内条目核对，也尚未合入 main。 diff --git a/internal/app/app.go b/internal/app/app.go index 364974c2..31b7bd0d 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -471,6 +471,7 @@ func New(cfg *config.Config, log *logger.Logger, configPath string) (*App, error externalMCPHandler := handler.NewExternalMCPHandler(externalMCPMgr, cfg, configPath, log.Logger) externalMCPHandler.SetAudit(auditSvc) roleHandler := handler.NewRoleHandler(cfg, configPath, log.Logger) + roleHandler.SetDB(db) roleHandler.SetAudit(auditSvc) skillsHandler := handler.NewSkillsHandler(cfg, configPath, log.Logger) skillsHandler.SetAudit(auditSvc) diff --git a/internal/handler/agent.go b/internal/handler/agent.go index 6f270e6b..ab30f360 100644 --- a/internal/handler/agent.go +++ b/internal/handler/agent.go @@ -763,9 +763,9 @@ func (h *AgentHandler) finalizeRobotAgentError(ctx context.Context, assistantMes func (h *AgentHandler) finalizeRobotAgentSuccess(taskCtx context.Context, assistantMessageID, conversationID string, resultMA *multiagent.RunResult) (string, string, error) { reasoningContent := multiagent.AggregatedReasoningFromTraceJSON(resultMA.LastAgentTraceInput) - decision := h.decideAgentRunForDeliveryWithPolicy(conversationID, assistantMessageID, "robot", resultMA, resultMA.MCPExecutionIDs, true) + decision := h.decideAgentRunForDeliveryWithPolicy(conversationID, assistantMessageID, "robot", resultMA, resultMA.MCPExecutionIDs, false) if cancelled := h.cleanupPendingToolExecutionsAfterIteration(taskCtx, conversationID, decision, nil); len(cancelled) > 0 { - decision = h.decideAgentRunForDeliveryWithPolicy(conversationID, assistantMessageID, "robot", resultMA, resultMA.MCPExecutionIDs, true) + decision = h.decideAgentRunForDeliveryWithPolicy(conversationID, assistantMessageID, "robot", resultMA, resultMA.MCPExecutionIDs, false) } h.persistFinalizationDecision(conversationID, assistantMessageID, "robot", resultMA.MCPExecutionIDs, reasoningContent, decision) responseText := decision.FinalText diff --git a/internal/handler/auth.go b/internal/handler/auth.go index 4157115e..c996af78 100644 --- a/internal/handler/auth.go +++ b/internal/handler/auth.go @@ -4,6 +4,7 @@ import ( "net/http" "strings" "time" + "unicode/utf8" "cyberstrike-ai/internal/audit" "cyberstrike-ai/internal/config" @@ -54,6 +55,11 @@ func (h *AuthHandler) Login(c *gin.Context) { c.JSON(http.StatusBadRequest, gin.H{"error": "密码不能为空"}) return } + req.Username = strings.TrimSpace(req.Username) + if req.Username == "" || utf8.RuneCountInString(req.Username) > 64 { + c.JSON(http.StatusBadRequest, gin.H{"error": "用户名不能为空，且不能超过 64 个字符"}) + return + } token, expiresAt, err := h.manager.Authenticate(req.Username, req.Password) if err != nil { diff --git a/internal/handler/auth_login_validation_test.go b/internal/handler/auth_login_validation_test.go new file mode 100644 index 00000000..72b3ece4 --- /dev/null +++ b/internal/handler/auth_login_validation_test.go @@ -0,0 +1,28 @@ +package handler + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gin-gonic/gin" +) + +func TestLoginRejectsMissingAndLongUsernameBeforeAuthentication(t *testing.T) { + h := &AuthHandler{} + r := gin.New() + r.POST("/login", h.Login) + for _, username := range []string{"", " ", strings.Repeat("x", 65), strings.Repeat("中", 65)} { + body, _ := json.Marshal(map[string]string{"username": username, "password": "test-only-password"}) + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/login", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Fatalf("username %q: %d", username, w.Code) + } + } +} diff --git a/internal/handler/batch_queue_executor.go b/internal/handler/batch_queue_executor.go index 8d93fc14..34ea2c49 100644 --- a/internal/handler/batch_queue_executor.go +++ b/internal/handler/batch_queue_executor.go @@ -302,10 +302,10 @@ func (h *AgentHandler) executeOneBatchSubTask(queueID string, queue *BatchTaskQu if useBatchMulti { agentMode = "batch_eino_" + batchOrch } - decision := h.decideAgentRunForDeliveryWithPolicy(conversationID, assistantMessageID, agentMode, resultMA, mcpIDs, true) + decision := h.decideAgentRunForDeliveryWithPolicy(conversationID, assistantMessageID, agentMode, resultMA, mcpIDs, false) autoCancelledPendingExecutionIDs := h.cleanupPendingToolExecutionsAfterIteration(taskCtx, conversationID, decision, progressCallback) if len(autoCancelledPendingExecutionIDs) > 0 { - decision = h.decideAgentRunForDeliveryWithPolicy(conversationID, assistantMessageID, agentMode, resultMA, mcpIDs, true) + decision = h.decideAgentRunForDeliveryWithPolicy(conversationID, assistantMessageID, agentMode, resultMA, mcpIDs, false) } h.persistFinalizationDecision(conversationID, assistantMessageID, agentMode, mcpIDs, reasoningContent, decision) resText := decision.FinalText diff --git a/internal/handler/c2.go b/internal/handler/c2.go index aa7d7fb9..dd1a0011 100644 --- a/internal/handler/c2.go +++ b/internal/handler/c2.go @@ -641,6 +641,10 @@ func (h *C2Handler) CreateTask(c *gin.Context) { // CancelTask 取消任务 func (h *C2Handler) CancelTask(c *gin.Context) { id := c.Param("id") + if !h.c2ResourceAllowed(c, "c2_task", id) { + c.JSON(http.StatusForbidden, gin.H{"error": "无权访问该资源"}) + return + } if err := h.mgr().CancelTask(id); err != nil { code := http.StatusInternalServerError if e, ok := err.(*c2.CommonError); ok { @@ -658,6 +662,10 @@ func (h *C2Handler) CancelTask(c *gin.Context) { // WaitTask 等待任务完成 func (h *C2Handler) WaitTask(c *gin.Context) { id := c.Param("id") + if !h.c2ResourceAllowed(c, "c2_task", id) { + c.JSON(http.StatusForbidden, gin.H{"error": "无权访问该资源"}) + return + } timeout := 60 * time.Second if t := c.Query("timeout"); t != "" { if n, err := strconv.Atoi(t); err == nil && n > 0 { diff --git a/internal/handler/c2_task_access_test.go b/internal/handler/c2_task_access_test.go new file mode 100644 index 00000000..467893d4 --- /dev/null +++ b/internal/handler/c2_task_access_test.go @@ -0,0 +1,74 @@ +package handler + +import ( + "net/http" + "net/http/httptest" + "path/filepath" + "testing" + "time" + + "cyberstrike-ai/internal/c2" + "cyberstrike-ai/internal/database" + "cyberstrike-ai/internal/security" + "github.com/gin-gonic/gin" + "go.uber.org/zap" +) + +func TestC2TaskWaitAndCancelRejectForeignTask(t *testing.T) { + db, err := database.NewDB(filepath.Join(t.TempDir(), "access.db"), zap.NewNop()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = db.Close() }) + if err := db.CreateC2Listener(&database.C2Listener{ID: "l_foreign", Name: "test", Type: "http_beacon", BindHost: "127.0.0.1", BindPort: 12345, Status: "stopped", CreatedAt: time.Now()}); err != nil { + t.Fatal(err) + } + if err := db.SetResourceOwner("c2_listener", "l_foreign", "owner"); err != nil { + t.Fatal(err) + } + if err := db.UpsertC2Session(&database.C2Session{ID: "s_foreign", ListenerID: "l_foreign", ImplantUUID: "test-uuid", Status: "active", FirstSeenAt: time.Now(), LastCheckIn: time.Now()}); err != nil { + t.Fatal(err) + } + task := &database.C2Task{ID: "t_foreign", SessionID: "s_foreign", TaskType: "exec", Status: "queued", CreatedAt: time.Now()} + if err := db.CreateC2Task(task); err != nil { + t.Fatal(err) + } + h := NewC2Handler(c2.NewManager(db, zap.NewNop(), t.TempDir()), zap.NewNop()) + r := gin.New() + r.Use(func(c *gin.Context) { + c.Set(security.ContextSessionKey, security.Session{UserID: "other", Scope: database.RBACScopeOwn}) + }) + r.GET("/tasks/:id/wait", h.WaitTask) + r.POST("/tasks/:id/cancel", h.CancelTask) + for _, req := range []*http.Request{ + httptest.NewRequest(http.MethodGet, "/tasks/t_foreign/wait", nil), + httptest.NewRequest(http.MethodPost, "/tasks/t_foreign/cancel", nil), + } { + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusForbidden { + t.Fatalf("%s: status=%d body=%s", req.URL, w.Code, w.Body.String()) + } + } + saved, err := db.GetC2Task(task.ID) + if err != nil || saved.Status != "queued" { + t.Fatalf("foreign task mutated: %#v, %v", saved, err) + } + // The same endpoints must still work for the listener's owner. + owned := gin.New() + owned.Use(func(c *gin.Context) { + c.Set(security.ContextSessionKey, security.Session{UserID: "owner", Scope: database.RBACScopeOwn}) + }) + owned.GET("/tasks/:id/wait", h.WaitTask) + owned.POST("/tasks/:id/cancel", h.CancelTask) + for _, req := range []*http.Request{ + httptest.NewRequest(http.MethodPost, "/tasks/t_foreign/cancel", nil), + httptest.NewRequest(http.MethodGet, "/tasks/t_foreign/wait", nil), + } { + w := httptest.NewRecorder() + owned.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("owner %s: status=%d body=%s", req.URL, w.Code, w.Body.String()) + } + } +} diff --git a/internal/handler/plain_text_finalization_test.go b/internal/handler/plain_text_finalization_test.go new file mode 100644 index 00000000..ed1562e3 --- /dev/null +++ b/internal/handler/plain_text_finalization_test.go @@ -0,0 +1,29 @@ +package handler + +import ( + "context" + "path/filepath" + "testing" + + "cyberstrike-ai/internal/database" + "cyberstrike-ai/internal/multiagent" + "go.uber.org/zap" +) + +func TestRobotDeliversPlainTextWithoutToolEvidence(t *testing.T) { + db, err := database.NewDB(filepath.Join(t.TempDir(), "robot.db"), zap.NewNop()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = db.Close() }) + conv, err := db.CreateConversation("text", database.ConversationCreateMeta{}) + if err != nil { + t.Fatal(err) + } + h := &AgentHandler{db: db, logger: zap.NewNop()} + result := &multiagent.RunResult{Response: "OK"} + text, _, err := h.finalizeRobotAgentSuccess(context.Background(), "", conv.ID, result) + if err != nil || text != "OK" || !result.Finalized { + t.Fatalf("text=%q finalized=%v error=%v", text, result.Finalized, err) + } +} diff --git a/internal/handler/role.go b/internal/handler/role.go index 1c061256..c5f17ed2 100644 --- a/internal/handler/role.go +++ b/internal/handler/role.go @@ -10,6 +10,7 @@ import ( "cyberstrike-ai/internal/audit" "cyberstrike-ai/internal/config" + "cyberstrike-ai/internal/database" "gopkg.in/yaml.v3" @@ -23,8 +24,11 @@ type RoleHandler struct { configPath string logger *zap.Logger audit *audit.Service + db *database.DB } +func (h *RoleHandler) SetDB(db *database.DB) { h.db = db } + // SetAudit wires platform audit logging. func (h *RoleHandler) SetAudit(s *audit.Service) { h.audit = s @@ -106,6 +110,16 @@ func (h *RoleHandler) UpdateRole(c *gin.Context) { if req.Name == "" { req.Name = roleName } + if err := h.validateRole(req); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + if req.Name != roleName { + if _, exists := h.config.Roles[req.Name]; exists { + c.JSON(http.StatusConflict, gin.H{"error": "角色已存在"}) + return + } + } // 初始化Roles map if h.config.Roles == nil { @@ -198,8 +212,8 @@ func (h *RoleHandler) CreateRole(c *gin.Context) { return } - if req.Name == "" { - c.JSON(http.StatusBadRequest, gin.H{"error": "角色名称不能为空"}) + if err := h.validateRole(req); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) return } diff --git a/internal/handler/role_validation.go b/internal/handler/role_validation.go new file mode 100644 index 00000000..1ab63e42 --- /dev/null +++ b/internal/handler/role_validation.go @@ -0,0 +1,57 @@ +package handler + +import ( + "fmt" + "strings" + "unicode" + "unicode/utf8" + + "cyberstrike-ai/internal/config" +) + +func validateRoleName(name string) error { + if strings.TrimSpace(name) == "" { + return fmt.Errorf("角色名称不能为空") + } + if name != strings.TrimSpace(name) || utf8.RuneCountInString(name) > 64 { + return fmt.Errorf("角色名称不能包含首尾空格，长度不能超过 64 个字符") + } + if strings.Contains(name, "..") || strings.ContainsAny(name, `/\:*?"<>|`) { + return fmt.Errorf("角色名称不能包含路径或文件名特殊字符") + } + for _, r := range name { + if unicode.IsControl(r) { + return fmt.Errorf("角色名称不能包含控制字符") + } + } + return nil +} + +func (h *RoleHandler) validateRole(role config.RoleConfig) error { + if err := validateRoleName(role.Name); err != nil { + return err + } + // Spaces are kept in display names but mapped to underscores on disk. + for key, existing := range h.config.Roles { + name := existing.Name + if name == "" { + name = key + } + if name != role.Name && strings.EqualFold(sanitizeFileName(name), sanitizeFileName(role.Name)) { + return fmt.Errorf("角色名称与已有角色的文件名冲突") + } + } + if strings.TrimSpace(role.WorkflowID) != "" { + if h.db == nil { + return fmt.Errorf("工作流存储不可用，无法验证绑定") + } + wf, err := h.db.GetWorkflowDefinition(role.WorkflowID) + if err != nil { + return fmt.Errorf("无法验证工作流绑定: %w", err) + } + if wf == nil { + return fmt.Errorf("角色绑定的工作流不存在") + } + } + return nil +} diff --git a/internal/handler/role_validation_test.go b/internal/handler/role_validation_test.go new file mode 100644 index 00000000..7e6ad49a --- /dev/null +++ b/internal/handler/role_validation_test.go @@ -0,0 +1,78 @@ +package handler + +import ( + "bytes" + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + + "cyberstrike-ai/internal/config" + "cyberstrike-ai/internal/database" + "github.com/gin-gonic/gin" + "go.uber.org/zap" +) + +func TestRoleNameValidation(t *testing.T) { + for _, name := range []string{"", " ", "../role", "a/b", `a\b`, "a..b", "a\n", strings.Repeat("中", 65)} { + if validateRoleName(name) == nil { + t.Fatalf("accepted %q", name) + } + } + for _, name := range []string{"安全分析", "Web Audit", "role-v2", strings.Repeat("中", 64)} { + if err := validateRoleName(name); err != nil { + t.Fatalf("rejected %q: %v", name, err) + } + } +} + +func TestRoleWorkflowValidationAndDeletion(t *testing.T) { + db, err := database.NewDB(filepath.Join(t.TempDir(), "roles.db"), zap.NewNop()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = db.Close() }) + if err := db.UpsertWorkflowDefinition(&database.WorkflowDefinition{ID: "wf", Name: "test", GraphJSON: `{"nodes":[],"edges":[]}`, Enabled: true}); err != nil { + t.Fatal(err) + } + cfg := &config.Config{Roles: map[string]config.RoleConfig{"bound": {Name: "bound", WorkflowID: "wf"}}} + h := NewRoleHandler(cfg, filepath.Join(t.TempDir(), "config.yaml"), zap.NewNop()) + h.SetDB(db) + wf := NewWorkflowHandler(db, zap.NewNop()) + wf.SetRuntime(nil, cfg) + r := gin.New() + r.POST("/roles", h.CreateRole) + r.PUT("/roles/:name", h.UpdateRole) + r.DELETE("/workflows/:id", wf.Delete) + for _, method := range []string{http.MethodPost, http.MethodPut} { + path := "/roles" + if method == http.MethodPut { + path += "/bound" + } + w := httptest.NewRecorder() + req := httptest.NewRequest(method, path, bytes.NewBufferString(`{"name":"new-role","workflow_id":"missing"}`)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Fatalf("%s: %d %s", method, w.Code, w.Body.String()) + } + } + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodDelete, "/workflows/wf", nil)) + if w.Code != http.StatusConflict { + t.Fatalf("delete referenced: %d %s", w.Code, w.Body.String()) + } + if err := h.validateRole(config.RoleConfig{Name: "valid", WorkflowID: "wf"}); err != nil { + t.Fatal(err) + } + if cfg.Roles["bound"].WorkflowID != "wf" { + t.Fatal("rejected update mutated roles") + } + delete(cfg.Roles, "bound") + w = httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodDelete, "/workflows/wf", nil)) + if w.Code != http.StatusOK { + t.Fatalf("delete unreferenced: %d %s", w.Code, w.Body.String()) + } +} diff --git a/internal/handler/workflow.go b/internal/handler/workflow.go index 2b9a45c9..a179ab44 100644 --- a/internal/handler/workflow.go +++ b/internal/handler/workflow.go @@ -243,6 +243,14 @@ func (h *WorkflowHandler) Delete(c *gin.Context) { c.JSON(http.StatusBadRequest, gin.H{"error": "工作流 id 不能为空"}) return } + if h.cfg != nil { + for _, role := range h.cfg.Roles { + if role.WorkflowID == id { + c.JSON(http.StatusConflict, gin.H{"error": "工作流仍被角色引用，请先解除绑定"}) + return + } + } + } if err := h.db.DeleteWorkflowDefinition(id); err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return diff --git a/web/static/i18n/en-US.json b/web/static/i18n/en-US.json index 955c7339..84bb0530 100644 --- a/web/static/i18n/en-US.json +++ b/web/static/i18n/en-US.json @@ -1965,6 +1965,7 @@ "forbidden": "Insufficient permissions", "requestFailed": "Request failed", "enterPassword": "Please enter password", + "usernameRequired": "Enter a username of at most 64 characters", "loginFailedCheck": "Sign-in failed, please check the username or password", "loginFailedRetry": "Sign-in failed, please try again later", "loggedOut": "Signed out" diff --git a/web/static/i18n/ru-RU.json b/web/static/i18n/ru-RU.json index 06f9aab1..a81ea311 100644 --- a/web/static/i18n/ru-RU.json +++ b/web/static/i18n/ru-RU.json @@ -1978,6 +1978,7 @@ "forbidden": "Недостаточно прав", "requestFailed": "Запрос не выполнен", "enterPassword": "Введите пароль", + "usernameRequired": "Введите имя пользователя длиной не более 64 символов", "loginFailedCheck": "Вход не выполнен, проверьте имя пользователя или пароль", "loginFailedRetry": "Вход не выполнен, повторите позже", "loggedOut": "Вы вышли" diff --git a/web/static/i18n/zh-CN.json b/web/static/i18n/zh-CN.json index cbe7cbda..c6bc729c 100644 --- a/web/static/i18n/zh-CN.json +++ b/web/static/i18n/zh-CN.json @@ -1953,6 +1953,7 @@ "forbidden": "权限不足", "requestFailed": "请求失败", "enterPassword": "请输入密码", + "usernameRequired": "用户名不能为空，且不能超过 64 个字符", "loginFailedCheck": "登录失败，请检查用户名或密码", "loginFailedRetry": "登录失败，请稍后重试", "loggedOut": "已退出登录" diff --git a/web/static/js/auth.js b/web/static/js/auth.js index 4245bfd5..ee8dfb59 100644 --- a/web/static/js/auth.js +++ b/web/static/js/auth.js @@ -257,6 +257,14 @@ async function submitLogin(event) { const username = usernameInput ? usernameInput.value.trim() : ''; const password = passwordInput.value.trim(); + if (!username || Array.from(username).length > 64) { + if (errorBox) { + errorBox.textContent = typeof window.t === 'function' ? window.t('auth.usernameRequired') : '用户名不能为空，且不能超过 64 个字符'; + errorBox.style.display = 'block'; + } + if (usernameInput) usernameInput.focus(); + return; + } if (!password) { if (errorBox) { const msgEmpty = (typeof window !== 'undefined' && typeof window.t === 'function') diff --git a/web/templates/index.html b/web/templates/index.html index bd66f4ec..102a6f20 100644 --- a/web/templates/index.html +++ b/web/templates/index.html @@ -53,7 +53,7 @@
- +