From 9137f6750761046b04e7adcfe29650eea6b13e93 Mon Sep 17 00:00:00 2001 From: Jeffery Date: Wed, 15 Jul 2026 10:25:23 +0800 Subject: [PATCH] =?UTF-8?q?chore(ai-review=20=E7=8B=80=E6=85=8B):=20?= =?UTF-8?q?=E6=B8=85=E7=A9=BA=20findings=20=E4=B8=A6=E7=99=BB=E8=A8=98?= =?UTF-8?q?=E6=8E=92=E5=BA=8F=20top-K=20=E5=BB=BA=E8=AD=B0=E7=82=BA?= =?UTF-8?q?=E8=AA=A4=E5=A0=B1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .gitea/ai-review/exclusions.json | 8 +++ .gitea/ai-review/findings.json | 99 +------------------------------- 2 files changed, 9 insertions(+), 98 deletions(-) diff --git a/.gitea/ai-review/exclusions.json b/.gitea/ai-review/exclusions.json index 6143972..b884c6a 100644 --- a/.gitea/ai-review/exclusions.json +++ b/.gitea/ai-review/exclusions.json @@ -134,5 +134,13 @@ "role": "Bard", "original_finding": "統一成同一套語彙，例如把 `section()` 改成 `setLogSection()`，並讓相關變數名稱也跟著一致。", "reason": "AI 對話收斂判定為誤報（問題在最新程式碼中不成立或不適用）" + }, + { + "location": "src/index.js:363", + "role": "Rogue", + "original_finding": "改用固定大小的 top-K 選擇策略，例如維持一個大小為 `KEEP_COUNT` 的最小堆，或在 API 已經有新到舊順序時直接取前 `KEEP_COUNT` 筆，避免整體排序。", + "reason": "release 數量預期不大，全量排序成本可忽略；後續同時需要「保留的前 K 筆」與「其餘待刪清單」，一次排序是最直接清楚的實作，引入 top-K 堆反而增加複雜度（與既有「分頁結果先完整收集到陣列」的排除理由一致）。", + "source": "develop...ai-review-resolve/develop-20260711-131608", + "date": "2026-07-15" } ] diff --git a/.gitea/ai-review/findings.json b/.gitea/ai-review/findings.json index 50f1939..fe51488 100644 --- a/.gitea/ai-review/findings.json +++ b/.gitea/ai-review/findings.json @@ -1,98 +1 @@ -[ - { - "level": "critical", - "role": "Mage", - "problem": "這裡在刪除 tag 的流程中只把失敗記進 `hadFailure`，但流程結束後沒有再檢查或拋錯。最小重現：只要任一個 tag 刪除回傳 404/500，程式仍會以 0 結束，外層 CI 會誤判為清理成功，但實際上遺留的 tag 還在。", - "suggestion": "在 `processInBatches(tagJson, ...)` 結束後補上 `if (hadFailure) throw new Error(...)`，讓任何 tag 刪除失敗都會正確回傳非 0 狀態。", - "location": "src/index.js:400", - "is_new": false - }, - { - "level": "warning", - "role": "Mage", - "problem": "`Promise.all` 只要其中一個刪除任務拋出 reject，整個 batch 會立刻失敗，外層 `main().catch(...)` 也會直接結束。這代表只要某一筆 release 或 tag 遇到網路中斷、DNS 失敗、連線逾時，後續同 batch 的項目就不會再處理，清理流程會在半途中停住。", - "suggestion": "把每個 item 的刪除包在個別 `try/catch`，或改用 `Promise.allSettled` 後統一彙總失敗；至少要確保單筆失敗不會中止同批其他清理工作。", - "location": "src/index.js:282", - "is_new": false - }, - { - "level": "warning", - "role": "Assassin", - "problem": "這個驗證只擋掉非數字，`0` 仍然會通過；若攻擊者能控制 `KEEP_COUNT`，就能把保留數設成 0，後續流程會把所有 release 刪光，還會把所有未被保留的 tag 一併清掉。", - "suggestion": "把下限改成至少 `1`，並在進入刪除流程前再做一次保護檢查；如果真的需要全清，應該改成獨立的高風險開關，而不是混在一般輸入參數裡。", - "location": "src/index.js:169", - "is_new": false - }, - { - "level": "warning", - "role": "Assassin", - "location": "src/index.js:252", - "problem": "`GITEA_REPOSITORY` 直接字串串進 release API 路徑，沒有做格式驗證或路徑編碼。只要這個值被污染，攻擊者就能把 `../`、額外斜線或其他路徑片段塞進去，讓帶著授權 token 的請求打到非預期的 API 路徑，擴大刪除面。", - "suggestion": "先把 repository 嚴格限制為 `owner/repo` 這種固定格式，再對 owner 與 repo 各自做 `encodeURIComponent` 後組 URL，不要直接把原字串拼進路徑。", - "is_new": true - }, - { - "level": "warning", - "role": "Assassin", - "location": "src/index.js:319", - "problem": "這裡同樣把未驗證的 `GITEA_REPOSITORY` 直接拼到 tag API 路徑。若輸入被操弄，攻擊者可以藉由路徑注入把刪除請求導向非預期資源，配合授權 token 造成超出原本 repo 範圍的破壞。", - "suggestion": "和 release API 一樣，對 repository 做嚴格格式檢查並逐段編碼後再組合路徑，必要時拒絕任何包含額外 `/`、`.` 或保留字元的值。", - "is_new": true - }, - { - "level": "warning", - "role": "Leo", - "location": "src/index.js:227", - "problem": "`processInBatches()` 用 `Promise.all` 搭配外層共享的 `hadFailure`，失敗語意會變得很難推理：一筆例外會直接中斷整批，但其他並行工作仍可能繼續跑，最後到底刪了哪些、漏了哪些，不看執行細節很難判斷。這種控制流對日後補測試或改錯誤處理都不友善。", - "suggestion": "把每筆處理的結果收斂成明確的成功/失敗回傳值，或在批次內逐筆捕捉錯誤後再彙總；若要保留並行，至少讓批次函式回傳可測試的結果集合，而不是依賴外部可變狀態。", - "is_new": true - }, - { - "level": "warning", - "role": "Mage", - "location": "src/index.js:345", - "problem": "`GITEA_SERVER_URL` 只做非空檢查，沒有先確認它是合法的絕對 URL。只要傳入像 `gitea.local`、`https://` 這類看起來有值但格式不合法的字串，`new URL()` 就會直接丟出未處理例外，錯誤也不會明確指出是參數格式問題。", - "suggestion": "在進入主流程前先對 `GITEA_SERVER_URL` 做 `try/catch` 驗證，失敗時回傳明確的參數錯誤並結束；不要把 URL 解析失敗留到中途才爆。", - "is_new": true - }, - { - "level": "warning", - "role": "Mage", - "location": "src/index.js:336", - "problem": "`KEEP_COUNT` 只驗證是數字字串，沒有保證落在安全整數範圍內。像 `9007199254740993` 這種值會在 `Number()` 轉換時失真，導致 `releaseCount <= keepCount` 與 `slice(keepCount)` 的保留/刪除判斷偏掉，最終清理結果可能和設定不一致。", - "suggestion": "除了字串格式外，還要驗證 `Number.isSafeInteger(Number(KEEP_COUNT))`，並加上合理上限；超出範圍時直接報錯，避免用不精確的數值做刪除決策。", - "is_new": true - }, - { - "level": "warning", - "role": "Rogue", - "location": "src/index.js:363", - "problem": "`releaseJson.sort(...)` 先把所有 release 做完整排序，成本是 O(n log n)，但後面其實只用前 `KEEP_COUNT` 筆。當 release 很多時，這段排序就是多花 CPU 在不必要的全量比較上。", - "suggestion": "改用固定大小的 top-K 選擇策略，例如維持一個大小為 `KEEP_COUNT` 的最小堆，或在 API 已經有新到舊順序時直接取前 `KEEP_COUNT` 筆，避免整體排序。", - "is_new": true - }, - { - "level": "warning", - "role": "Rogue", - "location": "src/index.js:230", - "problem": "這裡對每個 request 都先建立 `chunks` 陣列、收完整個 response body，再 `join` 成字串。刪除 release/tag 時大多只需要狀態碼，還硬把回應內容完整緩衝進記憶體，會在大量刪除時增加不必要的配置與拷貝。", - "suggestion": "把 request 包成可選擇是否收集 body；對 DELETE 這類不需要回應內容的呼叫直接丟棄資料串流，只保留 status code。", - "is_new": true - }, - { - "level": "info", - "role": "Assassin", - "problem": "分頁迴圈沒有上限，只要對方持續回傳非空頁面，這個 action 就會無限抓取；惡意或故障中的 API 可以把 runner 卡死，消耗時間與配額。", - "suggestion": "加入最大頁數、重複頁檢測或總筆數上限，超過就中止並回報異常，避免被外部回應拖成無限迴圈。", - "location": "src/index.js:228", - "is_new": false - }, - { - "level": "info", - "role": "Leo", - "problem": "在 `catch` 裡先把 `currentStage` 清空再記錄錯誤，會讓最後那筆失敗 log 失去「到底是在哪個階段炸掉」的上下文。等到未來有人要追問題時，只能回頭翻前面的輸出，比對成本會很高。", - "suggestion": "保留最後的 `currentStage`，或在進入 `catch` 時把階段一起寫進錯誤訊息；如果擔心汙染後續輸出，可以在輸出完成後再重設，而不是先清空。", - "location": "src/index.js:409", - "is_new": false - } -] +[]