fix(diagnostics): 處理 ai review findings #32
@@ -1989,5 +1989,16 @@
|
|||||||
"endLine": 188,
|
"endLine": 188,
|
||||||
"problem": "`resolveMergeBase` 新增了多階段 fetch 補歷史流程：先 fetch base、首次 merge-base、再 deepen base、deepen PR HEAD、必要時 unshallow，且每個策略成功後要立即重試並短路返回。現有測試只驗證不安全 `baseRef` 會在 fetch 前被拒絕，沒有測到淺層 checkout、首次失敗後補抓成功、策略失敗繼續下一個、全部失敗時診斷訊息等邊界。這正是容易 off-by-one 或順序錯的流程，現在還沒有測試保護。",
|
"problem": "`resolveMergeBase` 新增了多階段 fetch 補歷史流程：先 fetch base、首次 merge-base、再 deepen base、deepen PR HEAD、必要時 unshallow，且每個策略成功後要立即重試並短路返回。現有測試只驗證不安全 `baseRef` 會在 fetch 前被拒絕，沒有測到淺層 checkout、首次失敗後補抓成功、策略失敗繼續下一個、全部失敗時診斷訊息等邊界。這正是容易 off-by-one 或順序錯的流程，現在還沒有測試保護。",
|
||||||
"reason": "Paladin：可排除（重複）。resolveMergeBase 多階段 fetch、淺層與非淺層路徑、策略停止條件、全部失敗診斷與 cause 缺少測試，已由歷史 findings 涵蓋。"
|
"reason": "Paladin：可排除（重複）。resolveMergeBase 多階段 fetch、淺層與非淺層路徑、策略停止條件、全部失敗診斷與 cause 缺少測試，已由歷史 findings 涵蓋。"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"addedAt": "2026/07/21 16:26:12",
|
||||||
|
"prNumber": 6,
|
||||||
|
"reviewer": "Assassin",
|
||||||
|
"severity": "嚴重",
|
||||||
|
"file": "src/index.js",
|
||||||
|
"startLine": 105,
|
||||||
|
"endLine": 116,
|
||||||
|
"problem": "這裡把審查結果 commit/push 失敗吞掉並回傳 `false`，而本次變更又把主流程改成「本輪審查不因嚴重問題直接 exit 1，靠下一輪讀到 `[failure]` commit 才失敗」。攻擊者只要讓結果 commit 推不上去，例如在 PR head 競態推送、讓 token 沒有 push 權限、或讓來源分支拒絕 bot push，就能讓嚴重安全 finding 已產生但沒有 failure commit、也沒有下一輪失敗檢查，等同把必要檢查繞過。",
|
||||||
|
"reason": "人工裁示排除：主流程已在 result === failure 且 commitFindings 回傳 false 時直接回傳 1；本 finding 針對舊位置的描述已由現有收尾防線涵蓋。"
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
|
|||||||
@@ -7,84 +7,7 @@
|
|||||||
"version": "codex-cli 0.144.6",
|
"version": "codex-cli 0.144.6",
|
||||||
"model": "gpt-5.5"
|
"model": "gpt-5.5"
|
||||||
},
|
},
|
||||||
"findings": [
|
"findings": [],
|
||||||
{
|
|
||||||
"reviewer": "Assassin",
|
|
||||||
"focus": "security",
|
|
||||||
"badge": "🗡️",
|
|
||||||
"severity": "嚴重",
|
|
||||||
"file": "src/index.js",
|
|
||||||
"startLine": 105,
|
|
||||||
"endLine": 116,
|
|
||||||
"problem": "這裡把審查結果 commit/push 失敗吞掉並回傳 `false`，而本次變更又把主流程改成「本輪審查不因嚴重問題直接 exit 1，靠下一輪讀到 `[failure]` commit 才失敗」。攻擊者只要讓結果 commit 推不上去，例如在 PR head 競態推送、讓 token 沒有 push 權限、或讓來源分支拒絕 bot push，就能讓嚴重安全 finding 已產生但沒有 failure commit、也沒有下一輪失敗檢查，等同把必要檢查繞過。",
|
|
||||||
"suggestion": "嚴重問題存在時，結果 commit/push 失敗必須直接讓本輪 workflow 失敗；只有 success 結果或無變更時才可降級不阻擋。呼叫端應檢查 `commitFindings` 回傳值，或讓 `commitFindings` 在 `result === 'failure'` 時重拋錯誤。",
|
|
||||||
"suggestedCode": "const committed = commitFindings({ cwd, ctx, files: filesToCommit, result });\nif (result === 'failure' && !committed) {\n log('收尾', 'ERR', '存在嚴重問題，但無法推送 failure 結果 commit；本輪直接失敗以避免繞過檢查。');\n return 1;\n}\nreturn 0;",
|
|
||||||
"id": "F001",
|
|
||||||
"verdicts": {
|
|
||||||
"Paladin": {
|
|
||||||
"exclude": false,
|
|
||||||
"reason": "保留（成立）。已知排除事項與歷史 findings 未涵蓋「嚴重 finding 依賴 failure commit，但 failure commit 推送失敗時本輪仍可能通過」這個繞過風險；證據不足以排除。"
|
|
||||||
}
|
|
||||||
}
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"reviewer": "Bard",
|
|
||||||
"focus": "style",
|
|
||||||
"badge": "🎼",
|
|
||||||
"severity": "建議",
|
|
||||||
"file": "action.yml",
|
|
||||||
"startLine": 18,
|
|
||||||
"endLine": 19,
|
|
||||||
"problem": "中文敘述裡混入 `PR/issue`、`findings/exclusions` 這種半形斜線寫法，但同一份變更其他地方大量使用 `PR／issue`、`警告＋建議` 這類全形符號。標點像節拍器，這裡忽然換拍，讓 manifest 的文字風格不夠一致。",
|
|
||||||
"suggestion": "統一中文文件與註解中的分隔符號，建議在中文語境使用全形斜線：`PR／issue`、`findings／exclusions`；若是程式路徑或指令片段才保留半形 `/`。",
|
|
||||||
"suggestedCode": "",
|
|
||||||
"id": "F004",
|
|
||||||
"verdicts": {
|
|
||||||
"Paladin": {
|
|
||||||
"exclude": false,
|
|
||||||
"reason": "保留（成立）。未命中已知排除事項，也未見歷史 finding 涵蓋 action.yml 中文標點半形／全形風格不一致；依現有資料不能判為重複或誤報。"
|
|
||||||
}
|
|
||||||
}
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"reviewer": "Bard",
|
|
||||||
"focus": "style",
|
|
||||||
"badge": "🎼",
|
|
||||||
"severity": "建議",
|
|
||||||
"file": "src/lib/gitrepo.js",
|
|
||||||
"startLine": 299,
|
|
||||||
"endLine": 344,
|
|
||||||
"problem": "`pushWithCredential` 的 JSDoc 幾乎把安全設計、CI 觸發語意、checkout extraheader 行為全部寫成一篇短文。資訊本身有價值，但集中在函式註解裡會壓過函式簽名，讀者想找參數與責任邊界時，得先穿過一大段敘事。",
|
|
||||||
"suggestion": "保留函式層級的摘要與關鍵安全不變式，其餘背景可移到較短的段落或專門文件。JSDoc 建議聚焦在「做什麼、為何不能改、參數怎麼用」，避免把完整決策紀錄塞進 API 註解。",
|
|
||||||
"suggestedCode": "",
|
|
||||||
"id": "F005",
|
|
||||||
"verdicts": {
|
|
||||||
"Paladin": {
|
|
||||||
"exclude": false,
|
|
||||||
"reason": "保留（成立）。歷史 finding 雖有 resolveMergeBase 閱讀密度問題，但未涵蓋 pushWithCredential JSDoc 過長且混入決策紀錄；目前不能排除。"
|
|
||||||
}
|
|
||||||
}
|
|
||||||
},
|
|
||||||
{
|
|
||||||
"reviewer": "Leo",
|
|
||||||
"focus": "maintainability",
|
|
||||||
"badge": "🧰",
|
|
||||||
"severity": "建議",
|
|
||||||
"file": "src/lib/gitrepo.js",
|
|
||||||
"startLine": 367,
|
|
||||||
"endLine": 369,
|
|
||||||
"problem": "為了測試把內部函式掛在 `module.exports.__test`，會讓 production module 的公開形狀混入測試專用 API。未來其他程式碼可能誤用 `__test.assertSafeBranchRef`，而維護者也得在重構時顧慮這個非正式出口，模組邊界會慢慢變模糊。",
|
|
||||||
"suggestion": "把分支名稱驗證抽到獨立小模組並正常匯出，例如 `src/lib/gitref.js`，讓 production code 與測試都依賴同一個正式 API；或若它只屬於 gitrepo 內部，就改由測試 `resolveMergeBase`／`commitAndPushFindings` 的外部行為覆蓋，不暴露 `__test`。",
|
|
||||||
"suggestedCode": "",
|
|
||||||
"id": "F009",
|
|
||||||
"verdicts": {
|
|
||||||
"Paladin": {
|
|
||||||
"exclude": false,
|
|
||||||
"reason": "保留（成立）。目前提供的排除事項與歷史 findings 未涵蓋 module.exports.__test 暴露測試專用 API 的模組邊界問題；證據不足以排除。"
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
],
|
|
||||||
"excluded": [
|
"excluded": [
|
||||||
{
|
{
|
||||||
"reviewer": "Bard",
|
"reviewer": "Bard",
|
||||||
|
|||||||
Reference in New Issue
Block a user