From f6813cc2729895e1cdb543928cc416df78035a3e Mon Sep 17 00:00:00 2001 From: weichen Date: Tue, 15 Sep 2026 14:40:56 +0000 Subject: [PATCH] feat(api): add additive HTTP/SSE error envelopes Public errors now emit code, safe message, and request_id while keeping the legacy error field. Raw SQL, paths, tokens, and XSS payloads are stripped; the frontend branches on code instead of substring matching. Co-authored-by: Cursor --- ...5-http-error-envelope-h1-h3-test-report.md | 54 +++++++ internal/httpx/envelope.go | 123 ++++++++++++++ internal/httpx/envelope_test.go | 57 +++++++ internal/server/admin_permissions_handlers.go | 2 +- internal/server/admin_roles_handlers.go | 18 +-- internal/server/admin_template_handlers.go | 20 +-- internal/server/admin_workflow_handlers.go | 40 ++--- internal/server/asset_handlers.go | 22 +-- internal/server/auth_handlers.go | 68 ++++---- internal/server/auth_helpers.go | 6 +- internal/server/chat_digest_api.go | 14 +- internal/server/chat_export_api.go | 6 +- internal/server/chat_history_auth.go | 2 +- internal/server/chat_recategorize_api.go | 2 +- internal/server/chat_search_api.go | 8 +- internal/server/chat_turn_api.go | 8 +- internal/server/chat_unread_api.go | 10 +- internal/server/chat_window_api.go | 24 +-- internal/server/chatflow_http.go | 6 +- internal/server/freeform_doc_api.go | 16 +- internal/server/freeform_doc_api_test.go | 8 +- internal/server/handlers.go | 151 +++++++++--------- internal/server/me_settings_handlers.go | 14 +- internal/server/notification_handlers.go | 6 +- internal/server/piagent_e2e_test.go | 2 +- internal/server/project_doc_handlers.go | 12 +- internal/server/project_handlers.go | 60 +++---- internal/server/project_members_handlers.go | 32 ++-- internal/server/project_settings_handlers.go | 62 +++---- internal/server/project_workflow_api_test.go | 1 - internal/server/reconcile_handlers.go | 12 +- internal/server/reply_export_api.go | 4 +- internal/server/security_penetration_test.go | 10 +- internal/server/sse.go | 12 +- internal/server/task_run_auth.go | 8 +- internal/server/task_run_handlers.go | 36 ++--- internal/server/taskstream_test.go | 9 +- internal/server/workflow_catalog_handlers.go | 4 +- .../workflow_governance_piagent_pg_test.go | 6 +- internal/server/workflow_instance_handlers.go | 14 +- .../server/workflow_terminate_handlers.go | 24 +-- internal/stream/classify.go | 18 ++- internal/stream/classify_test.go | 11 ++ internal/stream/types.go | 61 +++---- web/src/App.tsx | 2 +- web/src/api/projectSettingsError.test.ts | 7 + web/src/api/projectSettingsError.ts | 3 +- web/src/hooks/useChatWindow.ts | 2 +- 48 files changed, 685 insertions(+), 412 deletions(-) create mode 100644 .doc/test-reports/2026-09-15-http-error-envelope-h1-h3-test-report.md create mode 100644 internal/httpx/envelope.go create mode 100644 internal/httpx/envelope_test.go diff --git a/.doc/test-reports/2026-09-15-http-error-envelope-h1-h3-test-report.md b/.doc/test-reports/2026-09-15-http-error-envelope-h1-h3-test-report.md new file mode 100644 index 00000000..4fc694b5 --- /dev/null +++ b/.doc/test-reports/2026-09-15-http-error-envelope-h1-h3-test-report.md @@ -0,0 +1,54 @@ +# HTTP/SSE 错误信封 H1–H3 测试报告 + +## 一、 测试背景 (Background) + +H0 盘点公开路由存在纯文本 `err.Error()`、JSON `{error,message}` 与 SSE `error_info` 三套错误体。本波 additive 统一信封，不改状态码。 + +## 二、 测试目标 (Test Objectives) + +- H1：`code`/`message`/`request_id`/`error` 双写；SQL/路径/token/XSS 不出现在响应。 +- H2：前端优先 `code`；未知 code 不靠 substring。 +- H3：去掉「无权访问该项目」文本匹配；不改状态码故无 ADR。 + +## 三、 测试方案与环境 (Test Plan & Environment) + +| 项 | 值 | +|----|-----| +| 分支 | `feature/http-error-envelope` | +| Go | `go test -short ./internal/httpx ./internal/stream ./internal/server` | +| 前端 | `npx tsx --test src/api/projectSettingsError.test.ts` | + +## 四、 预期结果 (Expected Results) + +信封 golden 负例 PASS；server 短测 PASS；前端解析 `code` 优先于 `error`。 + +## 五、 实际测试结果与判定 (Actual Results & Evaluation) + +### 5.1 httpx + SSE + +- **测试输入 (Input)**：`TestWriteSanitizesInternalAndKeepsLegacyCode`；`TestClassifyError_DoesNotLeakSQL` +- **实际输出 (Actual Output)**：`ok httpx`；`ok stream` +- **评价判定 (Evaluation / Verdict)**：**PASS** +- **性能与耗时 (Duration & Latency)**：<10ms + +### 5.2 server 回归 + +- **测试输入 (Input)**：`go test -short ./internal/server` +- **实际输出 (Actual Output)**：`ok 10.069s` +- **评价判定 (Evaluation / Verdict)**：**PASS** +- **性能与耗时 (Duration & Latency)**：10s + +### 5.3 前端 + +- **测试输入 (Input)**：`projectSettingsError.test.ts` +- **实际输出 (Actual Output)**：5 tests pass +- **评价判定 (Evaluation / Verdict)**：**PASS** +- **性能与耗时 (Duration & Latency)**：113ms + +## 六、 自动化测试套件汇总 (Test Suite Execution) + +上述均 PASS。未改 HTTP 状态码，无 ADR。 + +## 七、 测试结论与后续优化建议 (Conclusion & Next Steps) + +H1–H3 完成。`error` 字段仍作稳定 code 别名以兼容旧客户端。下一步 PI typed-result。 diff --git a/internal/httpx/envelope.go b/internal/httpx/envelope.go new file mode 100644 index 00000000..56c6fb52 --- /dev/null +++ b/internal/httpx/envelope.go @@ -0,0 +1,123 @@ +package httpx + +import ( + "crypto/rand" + "encoding/hex" + "encoding/json" + "net/http" + "strings" +) + +const ( + CodeAUTHZ = "AUTHZ" + CodeVALIDATION = "VALIDATION" + CodeVERSION_CONFLICT = "VERSION_CONFLICT" + CodeIDEMPOTENCY_CONFLICT = "IDEMPOTENCY_CONFLICT" + CodeINTERNAL = "INTERNAL" + HeaderRequestID = "X-Request-ID" +) + +type Envelope struct { + Code string `json:"code"` + Message string `json:"message"` + RequestID string `json:"request_id"` + Error string `json:"error"` + Details map[string]any `json:"details,omitempty"` +} + +func CodeFromStatus(status int) string { + switch status { + case http.StatusUnauthorized, http.StatusForbidden: + return CodeAUTHZ + case http.StatusConflict: + return CodeVERSION_CONFLICT + case http.StatusBadRequest, http.StatusUnprocessableEntity: + return CodeVALIDATION + default: + if status >= 500 { + return CodeINTERNAL + } + if status == http.StatusNotFound { + return CodeAUTHZ + } + return CodeVALIDATION + } +} + +func SafeMessage(code string) string { + switch code { + case CodeAUTHZ: + return "无权访问" + case CodeVALIDATION: + return "请求参数无效" + case CodeVERSION_CONFLICT: + return "资源版本冲突" + case CodeIDEMPOTENCY_CONFLICT: + return "重复提交" + default: + return "请求无法完成" + } +} + +func LooksInternal(s string) bool { + low := strings.ToLower(s) + needles := []string{"sql", "pq:", "postgres", "stack", "panic", "token", "password", "secret", "api_key", "bearer ", "../", "prompt"} + for _, n := range needles { + if strings.Contains(low, n) { + return true + } + } + if strings.Contains(s, "/home/") || strings.Contains(s, "/var/") || strings.Contains(s, "\\") { + return true + } + return false +} + +func RequestID(r *http.Request) string { + if r == nil { + return newRequestID() + } + if v := strings.TrimSpace(r.Header.Get(HeaderRequestID)); v != "" { + return v + } + return newRequestID() +} + +func newRequestID() string { + var b [8]byte + _, _ = rand.Read(b[:]) + return "req_" + hex.EncodeToString(b[:]) +} + +func PublicLegacy(code, legacy string) string { + legacy = strings.TrimSpace(legacy) + if legacy == "" || LooksInternal(legacy) { + return code + } + if strings.HasPrefix(legacy, "<") || strings.Contains(legacy, "alert(1)", "req_3", nil) + if strings.Contains(rec.Body.String(), " 0 { - http.Error(w, `{"error":"ERR_INVALID_PARAM","message":"无效的 JSON 请求体"}`, http.StatusBadRequest) + writeSafeError(w, r, http.StatusBadRequest, `{"error":"ERR_INVALID_PARAM","message":"无效的 JSON 请求体"}`) return } if req.StageKey == "" { diff --git a/internal/server/workflow_terminate_handlers.go b/internal/server/workflow_terminate_handlers.go index e03bba1c..ce391e7d 100644 --- a/internal/server/workflow_terminate_handlers.go +++ b/internal/server/workflow_terminate_handlers.go @@ -11,22 +11,22 @@ import ( func (s *Server) handleTerminationPreview(w http.ResponseWriter, r *http.Request) { if s.instSvc == nil || !s.instSvc.Available() { - http.Error(w, `{"error":"ERR_NOT_AVAILABLE"}`, http.StatusServiceUnavailable) + writeSafeError(w, r, http.StatusServiceUnavailable, `{"error":"ERR_NOT_AVAILABLE"}`) return } instanceID, err := strconv.ParseInt(r.PathValue("id"), 10, 64) if err != nil || instanceID <= 0 { - http.Error(w, `{"error":"ERR_INVALID_PARAM"}`, http.StatusBadRequest) + writeSafeError(w, r, http.StatusBadRequest, `{"error":"ERR_INVALID_PARAM"}`) return } user, err := s.authenticateUser(r) if err != nil || user == nil { - http.Error(w, `{"error":"ERR_UNAUTHENTICATED"}`, http.StatusUnauthorized) + writeSafeError(w, r, http.StatusUnauthorized, `{"error":"ERR_UNAUTHENTICATED"}`) return } detail, err := s.instSvc.GetInstanceDetail(r.Context(), instanceID) if err != nil { - http.Error(w, `{"error":"ERR_NOT_FOUND"}`, http.StatusNotFound) + writeSafeError(w, r, http.StatusNotFound, `{"error":"ERR_NOT_FOUND"}`) return } isCreator, _ := s.pwSvc.IsProjectCreator(r.Context(), detail.Instance.ProjectID, user.ID) @@ -40,27 +40,27 @@ func (s *Server) handleTerminationPreview(w http.ResponseWriter, r *http.Request func (s *Server) handleTerminateWorkflowInstance(w http.ResponseWriter, r *http.Request) { if s.instSvc == nil || !s.instSvc.Available() { - http.Error(w, `{"error":"ERR_NOT_AVAILABLE"}`, http.StatusServiceUnavailable) + writeSafeError(w, r, http.StatusServiceUnavailable, `{"error":"ERR_NOT_AVAILABLE"}`) return } instanceID, err := strconv.ParseInt(r.PathValue("id"), 10, 64) if err != nil || instanceID <= 0 { - http.Error(w, `{"error":"ERR_INVALID_PARAM"}`, http.StatusBadRequest) + writeSafeError(w, r, http.StatusBadRequest, `{"error":"ERR_INVALID_PARAM"}`) return } user, err := s.authenticateUser(r) if err != nil || user == nil { - http.Error(w, `{"error":"ERR_UNAUTHENTICATED"}`, http.StatusUnauthorized) + writeSafeError(w, r, http.StatusUnauthorized, `{"error":"ERR_UNAUTHENTICATED"}`) return } detail, err := s.instSvc.GetInstanceDetail(r.Context(), instanceID) if err != nil { - http.Error(w, `{"error":"ERR_NOT_FOUND"}`, http.StatusNotFound) + writeSafeError(w, r, http.StatusNotFound, `{"error":"ERR_NOT_FOUND"}`) return } var in projectinstance.TerminateInput if err := json.NewDecoder(r.Body).Decode(&in); err != nil { - http.Error(w, `{"error":"ERR_INVALID_PARAM"}`, http.StatusBadRequest) + writeSafeError(w, r, http.StatusBadRequest, `{"error":"ERR_INVALID_PARAM"}`) return } isCreator, _ := s.pwSvc.IsProjectCreator(r.Context(), detail.Instance.ProjectID, user.ID) @@ -68,11 +68,11 @@ func (s *Server) handleTerminateWorkflowInstance(w http.ResponseWriter, r *http. if err != nil { switch { case errors.Is(err, projectinstance.ErrForbiddenNotCreator): - http.Error(w, `{"error":"ERR_FORBIDDEN"}`, http.StatusForbidden) + writeSafeError(w, r, http.StatusForbidden, `{"error":"ERR_FORBIDDEN"}`) case errors.Is(err, projectinstance.ErrTerminationNotAllowed): - http.Error(w, `{"error":"ERR_TERMINATION_NOT_ALLOWED"}`, http.StatusConflict) + writeSafeError(w, r, http.StatusConflict, `{"error":"ERR_TERMINATION_NOT_ALLOWED"}`) default: - http.Error(w, `{"error":"ERR_INTERNAL","message":"`+err.Error()+`"}`, http.StatusInternalServerError) + writeSafeError(w, r, http.StatusInternalServerError, `{"error":"ERR_INTERNAL","message":"`+err.Error()+`"}`) } return } diff --git a/internal/stream/classify.go b/internal/stream/classify.go index 705f87c3..1d82e87f 100644 --- a/internal/stream/classify.go +++ b/internal/stream/classify.go @@ -15,14 +15,17 @@ func ClassifyError(err error, phaseID, requestID, sessionID, workspaceID string) } msg := strings.ToLower(err.Error()) - upstream := truncateRunes(err.Error(), 80) info := ErrorInfo{ PhaseID: phaseID, RequestID: requestID, SessionID: sessionID, WorkspaceID: workspaceID, - Upstream: upstream, + } + + raw := err.Error() + if !containsInternalLeak(raw) { + info.Upstream = truncateRunes(raw, 80) } switch { @@ -57,9 +60,20 @@ func ClassifyError(err error, phaseID, requestID, sessionID, workspaceID string) info.Actions = []string{"restart"} } + info.Message = info.UserMessage return info } +func containsInternalLeak(s string) bool { + low := strings.ToLower(s) + for _, n := range []string{"sql", "pq:", "token", "password", "/home/", "prompt"} { + if strings.Contains(low, n) || strings.Contains(s, n) { + return true + } + } + return false +} + func isTimeoutError(msg string) bool { return strings.Contains(msg, "timeout") || strings.Contains(msg, "deadline") || diff --git a/internal/stream/classify_test.go b/internal/stream/classify_test.go index 3242f34a..8e76a681 100644 --- a/internal/stream/classify_test.go +++ b/internal/stream/classify_test.go @@ -3,6 +3,7 @@ package stream import ( "errors" "net" + "strings" "testing" ) @@ -23,6 +24,16 @@ func TestClassifyError_TimeoutIsTransient(t *testing.T) { } } +func TestClassifyError_DoesNotLeakSQL(t *testing.T) { + info := ClassifyError(errors.New("pq: password authentication failed /home/weichen/x"), "dialogue_generating", "req_x", "sess_1", "") + if strings.Contains(info.Upstream, "pq:") || strings.Contains(info.Upstream, "/home/") || strings.Contains(info.UserMessage, "pq:") { + t.Fatalf("%+v", info) + } + if info.Message != info.UserMessage { + t.Fatal("message alias") + } +} + func TestClassifyError_GenericIsFatal(t *testing.T) { info := ClassifyError(errors.New("invalid project config"), "intent_recognizing", "req_2", "sess_1", "") if info.Class != ErrorClassFatal { diff --git a/internal/stream/types.go b/internal/stream/types.go index b92a0cf8..83aa6f0b 100644 --- a/internal/stream/types.go +++ b/internal/stream/types.go @@ -23,18 +23,18 @@ const ( // GovernancePayload 文档治理生命周期与认领变更载荷 type GovernancePayload struct { - DocID string `json:"doc_id"` - Action string `json:"action"` // claimed, proposal_required, proposal_decided, submitted_review, rejected, approved - CurrentState string `json:"current_state"` // drafting, reviewing, approving, approved - IsLocked bool `json:"is_locked"` - AuthorClaimed bool `json:"author_claimed"` - ProposalID string `json:"proposal_id,omitempty"` - ProposedChanges string `json:"proposed_changes,omitempty"` - ImpactSummary string `json:"impact_summary,omitempty"` - RiskSummary string `json:"risk_summary,omitempty"` - AuthorDecision string `json:"author_decision,omitempty"` - NotificationTitle string `json:"notification_title"` // 用户前台高亮简报标题 - NotificationMsg string `json:"notification_msg"` // 用户前台高亮简报内容 + DocID string `json:"doc_id"` + Action string `json:"action"` // claimed, proposal_required, proposal_decided, submitted_review, rejected, approved + CurrentState string `json:"current_state"` // drafting, reviewing, approving, approved + IsLocked bool `json:"is_locked"` + AuthorClaimed bool `json:"author_claimed"` + ProposalID string `json:"proposal_id,omitempty"` + ProposedChanges string `json:"proposed_changes,omitempty"` + ImpactSummary string `json:"impact_summary,omitempty"` + RiskSummary string `json:"risk_summary,omitempty"` + AuthorDecision string `json:"author_decision,omitempty"` + NotificationTitle string `json:"notification_title"` // 用户前台高亮简报标题 + NotificationMsg string `json:"notification_msg"` // 用户前台高亮简报内容 } // PhasePayload 阶段性任务状态更新载荷 @@ -66,12 +66,12 @@ type SummaryPayload struct { // DetailPayload 详细执行与推演过程载荷 (按需显示，默认折叠) type DetailPayload struct { - Category string `json:"category"` // agent_thought, step_output, tool_call, log - StepID string `json:"step_id,omitempty"` // 步骤标识 (如 tasks/01_regulatory) - Role string `json:"role,omitempty"` // 对应角色 - Title string `json:"title"` // 详情标题 - Content string `json:"content"` // 完整推演正文或技术草稿 - Collapsible bool `json:"collapsible"` // 客户端是否应当默认折叠显示 + Category string `json:"category"` // agent_thought, step_output, tool_call, log + StepID string `json:"step_id,omitempty"` // 步骤标识 (如 tasks/01_regulatory) + Role string `json:"role,omitempty"` // 对应角色 + Title string `json:"title"` // 详情标题 + Content string `json:"content"` // 完整推演正文或技术草稿 + Collapsible bool `json:"collapsible"` // 客户端是否应当默认折叠显示 } // DeliverablePayload A轨正式合规文档交付载荷 @@ -117,6 +117,7 @@ const ( type ErrorInfo struct { UserMessage string `json:"user_message"` + Message string `json:"message,omitempty"` Code string `json:"code"` Class ErrorClass `json:"class"` PhaseID string `json:"phase_id,omitempty"` @@ -133,16 +134,16 @@ type StreamEvent struct { Type EventType `json:"type"` Timestamp time.Time `json:"timestamp"` - Phase *PhasePayload `json:"phase,omitempty"` - Status *StatusPayload `json:"status,omitempty"` - Summary *SummaryPayload `json:"summary,omitempty"` - Briefing any `json:"briefing,omitempty"` - Detail *DetailPayload `json:"detail,omitempty"` - Delta string `json:"delta,omitempty"` - Deliverable *DeliverablePayload `json:"deliverable,omitempty"` - Governance *GovernancePayload `json:"governance,omitempty"` - Error string `json:"error,omitempty"` - ErrorInfo *ErrorInfo `json:"error_info,omitempty"` - Turn *TurnAckPayload `json:"turn,omitempty"` - Done *DonePayload `json:"done,omitempty"` + Phase *PhasePayload `json:"phase,omitempty"` + Status *StatusPayload `json:"status,omitempty"` + Summary *SummaryPayload `json:"summary,omitempty"` + Briefing any `json:"briefing,omitempty"` + Detail *DetailPayload `json:"detail,omitempty"` + Delta string `json:"delta,omitempty"` + Deliverable *DeliverablePayload `json:"deliverable,omitempty"` + Governance *GovernancePayload `json:"governance,omitempty"` + Error string `json:"error,omitempty"` + ErrorInfo *ErrorInfo `json:"error_info,omitempty"` + Turn *TurnAckPayload `json:"turn,omitempty"` + Done *DonePayload `json:"done,omitempty"` } diff --git a/web/src/App.tsx b/web/src/App.tsx index a6adf0e0..11d49cbb 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -2323,7 +2323,7 @@ export default function App() { }; } if (httpStatus === 403) { - if (code === 'ERR_FORBIDDEN' || detail.includes('无权访问该项目')) { + if (code === 'ERR_FORBIDDEN' || code === 'AUTHZ') { return { userMessage: detail || '无权访问该项目，请确认登录账号与项目成员身份', code: code || 'ERR_FORBIDDEN', diff --git a/web/src/api/projectSettingsError.test.ts b/web/src/api/projectSettingsError.test.ts index 0a5351fe..89375ab3 100644 --- a/web/src/api/projectSettingsError.test.ts +++ b/web/src/api/projectSettingsError.test.ts @@ -47,6 +47,13 @@ describe('projectSettingsError', () => { assert.equal(body.issues[0]?.code, 'DUTY_PARTIAL_ASSIGNMENT'); }); + it('prefers envelope code over legacy error field', () => { + const body = parseProjectSettingsErrorBody( + JSON.stringify({ code: 'AUTHZ', error: 'ERR_FORBIDDEN', message: '无权访问' }), + ); + assert.equal(body.code, 'AUTHZ'); + }); + it('retains the local draft when a late tab hits PLAN_FROZEN', () => { const draft = { workflow_definition_key: 'main', workflow_version: 1, stages: [] }; assert.equal(retainDraftOnFrozenConflict(draft), draft); diff --git a/web/src/api/projectSettingsError.ts b/web/src/api/projectSettingsError.ts index 4a3c151c..fa9fefbb 100644 --- a/web/src/api/projectSettingsError.ts +++ b/web/src/api/projectSettingsError.ts @@ -37,6 +37,7 @@ export function parseProjectSettingsErrorBody(text: string): { try { const parsed = JSON.parse(text) as { error?: string; + code?: string; message?: string; issues?: ProjectSettingsIssue[]; }; @@ -46,7 +47,7 @@ export function parseProjectSettingsErrorBody(text: string): { .filter((msg): msg is string => Boolean(msg)) .join('；'); return { - code: String(parsed.error || ''), + code: String(parsed.code || parsed.error || ''), message: issueText || parsed.message || parsed.error || text || '请求失败', issues, }; diff --git a/web/src/hooks/useChatWindow.ts b/web/src/hooks/useChatWindow.ts index f91805ac..47170dbb 100644 --- a/web/src/hooks/useChatWindow.ts +++ b/web/src/hooks/useChatWindow.ts @@ -69,7 +69,7 @@ function bucketHasMessages(days: ChatDayBucket[]): boolean { function isForbiddenResponse(err: unknown): boolean { if (!(err instanceof Error)) return false; const msg = err.message.toLowerCase(); - return msg.includes('403') || msg.includes('forbidden') || msg.includes('无权'); + return msg.includes('403') || msg.includes('forbidden'); } export function useChatWindow({