feat(ai-review 對話收斂): 讀 PR review 留言判斷解決狀態並收斂 findings #42
@@ -150,9 +150,10 @@ function isSafeRepoPath(p) {
|
||||
}
|
||||
|
||||
/**
|
||||
* 對話收斂主流程:取得 PR 所有行內 review comment、收斂成對話、跳過已 resolve 的,
|
||||
* 對所有「未解決」對話一律呼叫 Gitea resolve API 關閉(findings.json 為唯一待辦來源),
|
||||
* 再取最新程式碼交 AI 判斷每個對話的狀態並決定其在 findings 的去向:
|
||||
* 對話收斂主流程:取得 PR 所有行內 review comment,
|
||||
* 先把**每一個未解決的 comment**(依 comment id 去重,含無 path/position 者)一律呼叫 Gitea resolve API 關閉
|
||||
|
admin marked this conversation as resolved
Outdated
|
||||
* (findings.json 為唯一待辦來源,下次 review 依其重貼 comment);
|
||||
* 再以「檔案路徑+行號」收斂成對話、取最新程式碼交 AI 判斷,決定每個對話在 findings 的去向:
|
||||
* - 'resolved'(程式碼已修復)→ 從舊問題移除(resolvedFindings);
|
||||
* - 'false_positive'(誤報)→ 寫入 exclusions 並從舊問題移除(excludedFindings);
|
||||
* - 'open'(仍成立)→ 加入舊問題集合(carriedFindings)。
|
||||
@@ -177,8 +178,26 @@ export async function reconcileConversations(deps = {}) {
|
||||
const conversations = groupConversations(comments);
|
||||
const open = conversations.filter(c => !c.resolved && c.commentIds.length > 0);
|
||||
const alreadyResolved = conversations.length - open.length;
|
||||
|
admin marked this conversation as resolved
Outdated
admin
commented
嚴重等級:🔴 嚴重 **嚴重等級**:🔴 嚴重
**審查員**:Assassin
**問題**:在 `reconcileConversations` 函式中,從外部 Gitea comment 取得的 `c.path`(檔案路徑)未經額外驗證或淨化,直接傳遞給了 `getFileContent`(即 `getFileContentAtRef`)。由於 `getFileContentAtRef` 存在路徑穿越漏洞,攻擊者可以透過在 PR 中建立惡意檔案名稱,並在該檔案上留言,來觸發路徑穿越,讀取伺服器上的任意檔案。
**建議**:在將 `c.path` 傳遞給 `getFileContent` 之前,必須對其進行嚴格的白名單驗證,確保它只包含預期的檔案名稱字元,且不包含任何路徑穿越序列(例如 `..` 或 `/`)。或者,確保 `getFileContentAtRef` 的路徑處理是絕對安全的,不允許任何形式的路徑穿越。
admin
commented
嚴重等級:🔴 嚴重 **嚴重等級**:🔴 嚴重
**審查員**:Assassin
**問題**:在 `reconcileConversations` 函式中,從外部 Gitea comment 取得的 `c.path`(檔案路徑)未經額外驗證或淨化,直接傳遞給了 `getFileContent`(即 `getFileContentAtRef`)。由於 `getFileContentAtRef` 存在路徑穿越漏洞,攻擊者可以透過在 PR 中建立惡意檔案名稱,並在該檔案上留言,來觸發路徑穿越,讀取伺服器上的任意檔案。
**建議**:在將 `c.path` 傳遞給 `getFileContent` 之前,必須對其進行嚴格的白名單驗證,確保它只包含預期的檔案名稱字元,且不包含任何路徑穿越序列(例如 `..` 或 `/`)。或者,確保 `getFileContentAtRef` 的路徑處理是絕對安全的,不允許任何形式的路徑穿越。
|
||||
line(`對話收斂: 對話總數=${conversations.length} 已解決/不可處理=${alreadyResolved} 待處理=${open.length}`);
|
||||
if (open.length === 0) return { ...EMPTY };
|
||||
|
||||
// 要關閉的 comment:有 id 且尚未被 resolve(不依賴 path|line 分組,確保每個獨立 thread 都關到,含無 path/position 者)
|
||||
const unresolvedCommentIds = [...new Set(
|
||||
(comments || []).filter(c => c?.id != null && !c?.resolver).map(c => c.id),
|
||||
)];
|
||||
line(`對話收斂: 對話總數=${conversations.length} 已解決/不可處理=${alreadyResolved} 待判斷=${open.length} 待關閉 comment=${unresolvedCommentIds.length}`);
|
||||
|
||||
|
admin marked this conversation as resolved
Outdated
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Mage
**問題**:在 `reconcileConversations` 函式中,並行(`Promise.all`)呼叫 `resolveComment`,即使個別呼叫失敗,也僅在 `settled` 中記錄為 `rejected` 並印出 `warn`。然而,若 `resolveComment` 失敗是因為 `Authorization` token 過期或權限不足,後續所有的 `resolve` 呼叫都會失敗,此時程式碼沒有對這些特定的錯誤進行分類處理。
**建議**:應判斷 `outcome.reason` 的錯誤類型。若是連線/權限相關的嚴重錯誤,應立即停止後續的 `resolve` 嘗試,避免在已知無法成功的情況下發出無效請求。
|
||||
// 關閉所有未解決 comment(allSettled:個別失敗不中斷其他)
|
||||
const settled = await Promise.allSettled(unresolvedCommentIds.map(id => resolveComment(id)));
|
||||
let closedCount = 0;
|
||||
settled.forEach((s, i) => {
|
||||
if (s.status === 'fulfilled') closedCount += 1;
|
||||
else warn(`resolve comment 失敗: id=${unresolvedCommentIds[i]} error=${s.reason?.message}`);
|
||||
});
|
||||
if (unresolvedCommentIds.length > 0) ok(`已關閉 ${closedCount}/${unresolvedCommentIds.length} 個未解決 comment`);
|
||||
|
admin marked this conversation as resolved
Outdated
admin
commented
嚴重等級:🔵 建議 **嚴重等級**:🔵 建議
**審查員**:Mage
**問題**:對 `botFinding` 的存取缺乏防禦性檢查。
**建議**:在 `push` 之前增加防禦性檢查,確保物件完整性。
|
||||
|
||||
if (open.length === 0) {
|
||||
ok(`對話收斂完成: 關閉 comment=${closedCount} 已修復=0 誤報=0 仍成立=0`);
|
||||
return { ...EMPTY, closedCount };
|
||||
}
|
||||
|
||||
// 並行取得各檔案最新內容;單一檔案失敗時視為空字串,不中斷整體流程
|
||||
const fileCache = new Map();
|
||||
@@ -214,18 +233,6 @@ export async function reconcileConversations(deps = {}) {
|
||||
}
|
||||
const verdictByIdx = new Map(verdicts.map(v => [v.idx, v.verdict]));
|
||||
|
||||
// 全部先關閉:對所有未解決對話一律呼叫 resolve API(allSettled:個別失敗不中斷其他)
|
||||
const settled = await Promise.allSettled(open.map(c => resolveComment(c.commentIds[0])));
|
||||
let closedCount = 0;
|
||||
open.forEach((c, i) => {
|
||||
if (settled[i].status === 'fulfilled') {
|
||||
closedCount += 1;
|
||||
ok(`對話已關閉: ${c.path}:${c.line}`);
|
||||
} else {
|
||||
warn(`resolve 對話失敗: ${c.path}:${c.line} error=${settled[i].reason?.message}`);
|
||||
}
|
||||
});
|
||||
|
||||
// 依 AI 判斷決定每個對話在 findings 的去向
|
||||
const resolvedFindings = []; // 已修復 → 從舊問題移除
|
||||
const excludedFindings = []; // 誤報 → 寫入 exclusions 並從舊問題移除
|
||||
@@ -248,7 +255,7 @@ export async function reconcileConversations(deps = {}) {
|
||||
}
|
||||
}
|
||||
|
||||
ok(`對話收斂完成: 關閉對話=${closedCount}/${open.length} 已修復=${resolvedCount} 誤報=${falsePositiveCount} 仍成立=${openCount}`);
|
||||
ok(`對話收斂完成: 關閉 comment=${closedCount}/${unresolvedCommentIds.length} 已修復=${resolvedCount} 誤報=${falsePositiveCount} 仍成立=${openCount}`);
|
||||
return {
|
||||
resolvedFindings, excludedFindings, carriedFindings,
|
||||
resolvedCount, falsePositiveCount, openCount, closedCount,
|
||||
|
||||
嚴重等級:🔴 嚴重
審查員:Rogue
問題:這裡又在浪費時間!
reconcileConversations函式在取得所有獨特的檔案路徑後,又在迴圈裡對每個檔案路徑依序呼叫getFileContent。如果有很多檔案需要檢查,這會導致F次遠端 API 呼叫依序執行,嚴重拖慢整體流程。建議:改用
Promise.all或Promise.allSettled來並行發送所有getFileContent的請求。這樣可以大幅減少等待時間,讓檔案內容的取得幾乎同時完成。