feat(ai-review 對話收斂): 讀 PR review 留言判斷解決狀態並收斂 findings #45
@@ -398,5 +398,41 @@
|
|||||||
"role": "Leo",
|
"role": "Leo",
|
||||||
"original_finding": "將 `REVIEW_SEVERITY_LABELS`、`REVIEW_SEVERITY_PATTERN` 和 `reviewSeverityLabel` 這些與評論格式相關的常數與函式,提取到一個獨立的共用模組中(例如 `app/utils/reviewComments.js`),並讓測試檔案和任何需要用到它們的應用程式邏輯都從該模組匯入。這樣能確保「評論格式」的定義只有一個來源,提升可維護性。",
|
"original_finding": "將 `REVIEW_SEVERITY_LABELS`、`REVIEW_SEVERITY_PATTERN` 和 `reviewSeverityLabel` 這些與評論格式相關的常數與函式,提取到一個獨立的共用模組中(例如 `app/utils/reviewComments.js`),並讓測試檔案和任何需要用到它們的應用程式邏輯都從該模組匯入。這樣能確保「評論格式」的定義只有一個來源,提升可維護性。",
|
||||||
"reason": "誤判。這些常數與 `reviewSeverityLabel` 只用於 `app/comments.test.js` 內部驗證 review comment body 格式,production code 沒有使用同一段解析邏輯;抽成共用模組會把測試專用輔助程式提升為正式 API,增加不必要的維護負擔。"
|
"reason": "誤判。這些常數與 `reviewSeverityLabel` 只用於 `app/comments.test.js` 內部驗證 review comment body 格式,production code 沒有使用同一段解析邏輯;抽成共用模組會把測試專用輔助程式提升為正式 API,增加不必要的維護負擔。"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"location": "app/resolve.js:52",
|
||||||
|
"role": "Assassin",
|
||||||
|
"original_finding": "在 `parseBotReviewComment` 函式中,從 Gitea comment 內文解析出的 `problem` 和 `suggestion` 欄位若包含惡意內容且未經適當輸出編碼,可能導致 XSS 攻擊。",
|
||||||
|
"reason": "誤判。這些字串只會寫入 `.gitea/ai-review/findings.json` 與 Gitea review comment body,Gitea 的 Markdown 渲染器會在伺服器端對輸出做 HTML 淨化;本 action 不自行將其渲染到任何自製網頁或 UI,輸出編碼屬消費端(Gitea)責任。"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"location": "app/resolve.js:207",
|
||||||
|
"role": "Leo",
|
||||||
|
"original_finding": "正規化邏輯(`normalizeKey` 等)過於激進且未快取,既可能導致語意相近建議被誤判為相同,也在頻繁比較時造成效能浪費。",
|
||||||
|
"reason": "誤判/過度設計。積極正規化是刻意設計,用來對行號漂移與標點差異產生穩定簽章以利去重;`findingSig` 已是獨立 helper,且比對對象為單一 PR 的小量 findings,memoize 在此規模沒有實質效益。"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"location": "app/resolve.js:187",
|
||||||
|
"role": "Mage",
|
||||||
|
"original_finding": "在 `reconcileConversations` 函式中,並行(`Promise.all`)呼叫 `resolveComment`,即使個別呼叫失敗也僅記錄為 rejected 並 warn;若失敗是 token 過期或權限不足,後續所有 resolve 都會失敗,程式碼未對這些錯誤分類並提前停止。",
|
||||||
|
"reason": "不適用。resolve 呼叫已改為 `Promise.allSettled` 一次並行送出,不存在「後續逐一嘗試」可中止;個別失敗已降級記錄並把該對話保留為未解決,不影響其他對話與整體流程。"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"location": "app/resolve.js:77",
|
||||||
|
"role": "Rogue",
|
||||||
|
"original_finding": "大量使用字串拼接產生暫存物件,以及並行請求未限制數量,在高負載下可能導致 GC 壓力或觸發 API 限流;建議引入 p-limit 等並行限制。",
|
||||||
|
"reason": "過度設計。對話來源為單一 PR 的行內 review comment,數量級小,無限並行不致造成 GC 壓力或觸發限流;引入 p-limit 相依與額外複雜度在此情境不符成本效益。"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"location": "app/resolve.js:195",
|
||||||
|
"role": "Mage",
|
||||||
|
"original_finding": "對 `botFinding` 的存取缺乏防禦性檢查。",
|
||||||
|
"reason": "誤判。`pushCarried` 進入時即有 `if (!conversation.botFinding) return` 防禦,resolvedFindings 的 push 也有 `if (c.botFinding)` 判斷,存取前皆已檢查物件存在。"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"location": "app/resolve.js:173",
|
||||||
|
"role": "Rogue",
|
||||||
|
"original_finding": "`Promise.allSettled` 的結果處理邏輯過於冗長,產生不必要的中間變數。",
|
||||||
|
"reason": "主觀風格。`resolveOutcome` Map 是為了讓並行結果能依原 open 索引亂序對齊(保留 carried/resolved 的順序與 botFinding 對應),現有寫法清楚且正確,非缺陷。"
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
|
|||||||
@@ -1,109 +1 @@
|
|||||||
[
|
[]
|
||||||
{
|
|
||||||
"level": "critical",
|
|
||||||
"role": "Assassin",
|
|
||||||
"location": "app/resolve.js:180",
|
|
||||||
"problem": "在 `reconcileConversations` 函式中,從外部 Gitea comment 取得的 `c.path`(檔案路徑)未經額外驗證或淨化,直接傳遞給了 `getFileContent`(即 `getFileContentAtRef`)。由於 `getFileContentAtRef` 存在路徑穿越漏洞,攻擊者可以透過在 PR 中建立惡意檔案名稱,並在該檔案上留言,來觸發路徑穿越,讀取伺服器上的任意檔案。",
|
|
||||||
"suggestion": "在將 `c.path` 傳遞給 `getFileContent` 之前,必須對其進行嚴格的白名單驗證,確保它只包含預期的檔案名稱字元,且不包含任何路徑穿越序列(例如 `..` 或 `/`)。或者,確保 `getFileContentAtRef` 的路徑處理是絕對安全的,不允許任何形式的路徑穿越。",
|
|
||||||
"is_new": false
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "critical",
|
|
||||||
"role": "Assassin",
|
|
||||||
"location": "app/resolve.js:145",
|
|
||||||
"problem": "LLM 提示詞注入風險:在 `judgeConversationsResolved` 函式中,外部來源的 `thread` 和 `code` 被直接拼接進傳給 LLM 的 `payload` 中,攻擊者可能注入惡意指令來劫持 LLM 行為。",
|
|
||||||
"suggestion": "對所有傳遞給 LLM 的外部輸入進行嚴格的淨化和隔離。使用結構化輸入而非直接拼接字串,並在提示詞中明確指示 AI 忽略任何試圖下達指令的內容,僅對邏輯進行判斷。對於敏感操作,應建立多層驗證機制。"
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "critical",
|
|
||||||
"role": "Mage",
|
|
||||||
"location": "app/resolve.js:77",
|
|
||||||
"problem": "在 `judgeConversationsResolved` 函式中,對 `chatFn` 的結果結構缺乏足夠的嚴格檢查。若回傳結構不符合預期,可能導致所有對話被錯誤判定為「未解決」。",
|
|
||||||
"suggestion": "增加對 `result` 結構的嚴格檢查。如果 `result` 不是預期的陣列結構,應拋出例外或進行更謹慎的錯誤處理,而不是默默地將所有對話視為未解決。"
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Assassin",
|
|
||||||
"location": "app/resolve.js:52",
|
|
||||||
"problem": "在 `parseBotReviewComment` 函式中,從 Gitea comment 內文解析出的 `problem` 和 `suggestion` 欄位若包含惡意內容且未經適當輸出編碼,可能導致 XSS 攻擊。",
|
|
||||||
"suggestion": "確保所有從外部來源解析出的字串在渲染到任何介面時,都必須經過嚴格的上下文相關輸出編碼(例如 HTML 實體編碼),以防止 XSS 攻擊。"
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Leo",
|
|
||||||
"location": "app/resolve.js:207",
|
|
||||||
"problem": "正規化邏輯(`normalizeKey` 等)過於激進且未快取,既可能導致語意相近建議被誤判為相同,也在頻繁比較時造成效能浪費。",
|
|
||||||
"suggestion": "請評估目前的正規化規則,若發現誤判,放寬規則或加入關鍵字比對。將簽章產生邏輯抽離為獨立 Helper 函式,並在產生時進行快取(Memoize)以提升效能。"
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Maya",
|
|
||||||
"location": "app/resolve.js:144",
|
|
||||||
"problem": "缺少關鍵邊界條件與異常路徑的測試案例。包含 `judge` 拋出錯誤、`chatFn` 解析異常、`levelRaw` 或 `suggestion` 空值、`getFileContent` 失敗以及混合正確/錯誤的判斷數據等場景。",
|
|
||||||
"suggestion": "請在 `app/resolve.test.js` 中新增這些邊界條件的測試案例,確保系統在面對 AI 異常輸出、API 失敗、或輸入欄位缺失時,仍能穩健處理並符合預期行為。"
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Assassin",
|
|
||||||
"location": "app/resolve.js:18",
|
|
||||||
"problem": "函式 `parseBotReviewComment` 動態產生正規表達式,且輸入來源 `body` 為外部輸入,存在 Regex Injection 風險。",
|
|
||||||
"suggestion": "將正規表達式改為靜態定義,並透過 `String.raw` 或更安全的字串處理方式來匹配標籤,確保輸入不包含特殊 regex 字元。"
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Bard",
|
|
||||||
"location": "app/resolve.js",
|
|
||||||
"problem": "`judgeConversationsResolved` 內的 `systemPrompt` 硬編碼在函式中,過於冗長且干擾邏輯。",
|
|
||||||
"suggestion": "將此 `systemPrompt` 抽離為檔案層級的常數,提升程式碼結構美與清晰度。"
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Leo",
|
|
||||||
"location": "app/resolve.js:77",
|
|
||||||
"problem": "程式碼片段定位邏輯(如字串拼接行號)與上下文擷取策略(如 radius)寫死在函式內,擴展性與維護性不足。",
|
|
||||||
"suggestion": "建立明確的 `Location` 物件封裝定位資訊,並將 `radius` 或擷取策略抽離為配置參數或常數。"
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Mage",
|
|
||||||
"location": "app/resolve.js:187",
|
|
||||||
"problem": "在 `reconcileConversations` 函式中,並行(`Promise.all`)呼叫 `resolveComment`,即使個別呼叫失敗,也僅在 `settled` 中記錄為 `rejected` 並印出 `warn`。然而,若 `resolveComment` 失敗是因為 `Authorization` token 過期或權限不足,後續所有的 `resolve` 呼叫都會失敗,此時程式碼沒有對這些特定的錯誤進行分類處理。",
|
|
||||||
"suggestion": "應判斷 `outcome.reason` 的錯誤類型。若是連線/權限相關的嚴重錯誤,應立即停止後續的 `resolve` 嘗試,避免在已知無法成功的情況下發出無效請求。",
|
|
||||||
"is_new": true
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Rogue",
|
|
||||||
"location": "app/resolve.js:77",
|
|
||||||
"problem": "大量使用字串拼接產生暫存物件,以及並行請求未限制數量,在高負載下可能導致 GC 壓力或觸發 API 限流。",
|
|
||||||
"suggestion": "對於大量 comments,考慮使用複合物件或分層 Map 結構。引入請求並行限制(如 `p-limit`)來確保系統穩定性。"
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "info",
|
|
||||||
"role": "Mage",
|
|
||||||
"location": "app/resolve.js:70",
|
|
||||||
"problem": "缺乏位置資訊的留言會被歸類到同一個預設 key,可能導致不相關留言被錯誤歸併。",
|
|
||||||
"suggestion": "明確過濾缺乏 `path` 或 `position` 的留言,或提供更具區分性的預設 key。"
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "info",
|
|
||||||
"role": "Bard",
|
|
||||||
"location": "app/resolve.js:8",
|
|
||||||
"problem": "RegExp 在函式內部重複建立,造成不必要的效能損耗。",
|
|
||||||
"suggestion": "將正則表達式移至函式外部宣告為常數。"
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "info",
|
|
||||||
"role": "Mage",
|
|
||||||
"location": "app/resolve.js:195",
|
|
||||||
"problem": "對 `botFinding` 的存取缺乏防禦性檢查。",
|
|
||||||
"suggestion": "在 `push` 之前增加防禦性檢查,確保物件完整性。"
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"level": "info",
|
|
||||||
"role": "Rogue",
|
|
||||||
"location": "app/resolve.js:173",
|
|
||||||
"problem": "`Promise.allSettled` 的結果處理邏輯過於冗長,產生不必要的中間變數。",
|
|
||||||
"suggestion": "優化處理邏輯,直接在迴圈內處理或使用更緊湊的寫法。"
|
|
||||||
}
|
|
||||||
]
|
|
||||||
|
|||||||
Reference in New Issue
Block a user