From 7b742ed85e2afedc8d0ca5ee0a5b0ffdebb3509f Mon Sep 17 00:00:00 2001 From: Jeffery Date: Fri, 26 Jun 2026 13:55:48 +0800 Subject: [PATCH] =?UTF-8?q?chore(ai-review):=20=E6=9B=B4=E6=96=B0=20findin?= =?UTF-8?q?gs.json=20=E8=88=87=20exclusions.json?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 移除本輪已修復(server URL 正規化、id 整數驗證、整合測試)與重複提報的 finding; 保留 7 條設計取捨(重試/錯誤處理/DoS/TOCTOU/deleteResource 策略); 新增 3 條不適用至 exclusions(baseUrl 由已驗證 config 組成、tag 白名單非必要、 categorizeTags 拆分無益)。 Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitea/ai-review/exclusions.json | 18 +++++++ .gitea/ai-review/findings.json | 86 +++++++++----------------------- 2 files changed, 42 insertions(+), 62 deletions(-) diff --git a/.gitea/ai-review/exclusions.json b/.gitea/ai-review/exclusions.json index ae4eff0..4607f96 100644 --- a/.gitea/ai-review/exclusions.json +++ b/.gitea/ai-review/exclusions.json @@ -106,5 +106,23 @@ "role": "Leo", "original_finding": "建議在 logger.js 建立通用 log(level, prefix, stream) 函式,讓 info/warn/fail 等共用以降低重複。", "reason": "不採納。六個輸出函式皆為單行、語意直觀;為此抽象通用 dispatcher 反而增加間接層與閱讀成本,去重效益微小。" + }, + { + "location": "app/gitea-client.js:63", + "role": "Assassin", + "original_finding": "baseUrl 直接拼接到 URL,若來自惡意環境變數恐 SSRF;建議確保 baseUrl 只能以預期的 GITEA_SERVER_URL 開頭。", + "reason": "不適用。fetchAllPages 的 baseUrl 並非外部輸入,而是由 config 的 releaseApiUrl/tagApiUrl 提供,二者皆以已通過 requireUrl 驗證的 GITEA_SERVER_URL 與通過 requireRepository 驗證的 repository 組成 `${serverUrl}/api/v1/repos/${repository}/...`,本即保證以 GITEA_SERVER_URL 開頭,無額外白名單必要。" + }, + { + "location": "app/tags.js:62", + "role": "Assassin", + "original_finding": "tag 名稱拼接至 URL 雖用 encodeURIComponent,仍建議以白名單格式驗證(限英數字、點、破折號,禁 .. 或 /)以更安全。", + "reason": "不採納。encodeURIComponent 會將路徑分隔字元 / 編碼為 %2F,已消除 URL 路徑穿越;且 git ref 命名規則本即禁止 tag 含 '..'、控制字元與多數特殊字元。額外白名單反而可能誤拒合法 tag 名,且非 URL 安全所必需。" + }, + { + "location": "app/tags.js:19", + "role": "Bard", + "original_finding": "categorizeTags 內部判斷邏輯稍複雜,建議拆分為更小的判斷函式以提高可讀性。", + "reason": "不採納。categorizeTags 僅為 skip/keep/delete 三分支的單層 map,語意已清楚且具完整測試;為三個簡單條件再抽出微函式只會增加跳轉與閱讀成本,可讀性無實質提升。" } ] diff --git a/.gitea/ai-review/findings.json b/.gitea/ai-review/findings.json index 88660b0..c781aa1 100644 --- a/.gitea/ai-review/findings.json +++ b/.gitea/ai-review/findings.json @@ -1,18 +1,4 @@ [ - { - "level": "critical", - "role": "Assassin", - "location": "app/gitea-client.js:63", - "problem": "攻擊者可控制 baseUrl 參數，若未對 baseUrl 的來源進行嚴格限制，可能導致 Server-Side Request Forgery (SSRF) 攻擊。此處直接將變數拼接到 URL，如果 baseUrl 來自惡意環境變數，攻擊者能發送請求到內網資源或意圖控制的位址。", - "suggestion": "應在 config.js 中對 GITEA_SERVER_URL 和 GITEA_REPOSITORY 進行嚴格的 URL 結構驗證與白名單過濾，確保拼接出的 URL 處於預期範圍內。目前雖有 requireUrl 和 requireRepository，應進一步強化確保 baseUrl 只能以預期的 GITEA_SERVER_URL 開頭。" - }, - { - "level": "critical", - "role": "Maya", - "location": "app/index.js:21", - "problem": "Action 的核心邏輯 cleanupReleases 與 cleanupOrphanTags 被直接呼叫，但缺少針對清理流程的整合測試或端對端測試，僅有單元測試無法保證整個「清理 -> 再清理 tag」的完整路徑是否會因環境設定或 API 回應產生非預期的行為。", - "suggestion": "建議補上一個整合測試 (app/test/integration.test.js)，模擬完整的 API 回應序列（如：先列出舊版本 -> 刪除舊版本 -> 重新列出 release -> 列出 tag -> 刪除孤立 tag），驗證所有 API 呼叫順序與參數皆符合預期。" - }, { "level": "warning", "role": "Mage", @@ -20,8 +6,16 @@ "problem": "清理流程對網路請求依賴強,若 API 呼叫失敗,整個 main 流程中斷,無法確保後續清理的一致性與部分成功重試。", "suggestion": "引入更細緻的錯誤處理(如錯誤閾值機制)或部分清理成功後的重試策略。", "status": "deferred", - "defer_reason": "錯誤處理/重試策略的設計取捨。目前單筆刪除失敗會記錄並繼續、讀取失敗則中止屬合理保守行為;是否引入閾值/重試保留待人工評估。", - "is_new": false + "defer_reason": "錯誤處理/重試策略的設計取捨。目前單筆刪除失敗會記錄並繼續、讀取失敗則中止屬合理保守行為;是否引入閾值/重試保留待人工評估。" + }, + { + "level": "warning", + "role": "Maya", + "location": "app/releases.js:46", + "problem": "cleanupReleases 中若 fetchAllPages 拋錯,cleanupOrphanTags 不會執行;缺乏部分失敗後的清理與回報機制。", + "suggestion": "考慮在 cleanupReleases 加入 try/catch,僅該步驟失敗時記錄並仍嘗試 cleanupOrphanTags,或明確標示部分清理狀態。", + "status": "deferred", + "defer_reason": "目前採 fail-fast:無法取得 release 清單時不應依過時資料刪除 tag,中止較安全;且 fetchAllPages 的錯誤訊息已含失敗的 GET URL,可辨識中斷步驟。是否改為跨步驟續行屬設計取捨,保留待人工評估。" }, { "level": "warning", @@ -30,8 +24,7 @@ "problem": "錯誤路徑以 res.text() 讀取整個回應主體,未限制大小,惡意伺服器可回傳極大內容導致記憶體耗盡(DoS)。", "suggestion": "限制讀取的回應大小,例如檢查 Content-Length 或以串流方式設定讀取上限。", "status": "deferred", - "defer_reason": "風險低:目標為已通過 URL 驗證的受信任 Gitea 實例,且每請求已有 30 秒逾時。正確修法需串流逐段讀取並設位元組上限,屬較大改動,保留待人工評估。", - "is_new": false + "defer_reason": "風險低:目標為已通過 URL 驗證的受信任 Gitea 實例,且每請求已有 30 秒逾時。正確修法需串流逐段讀取並設位元組上限,屬較大改動,保留待人工評估。" }, { "level": "warning", @@ -40,64 +33,33 @@ "problem": "成功路徑以 res.json() 解析整個回應,未限制大小,惡意伺服器可回傳極大 JSON 導致記憶體耗盡(DoS)。", "suggestion": "對 API 回應設定明確大小上限,超過時拒絕解析並拋出異常。", "status": "deferred", - "defer_reason": "與 app/gitea-client.js:66 同類:受信任目標、已有逾時與 MAX_PAGES 約束,風險低;正確修法需串流讀取並設上限,屬較大改動,保留待人工評估。", - "is_new": false + "defer_reason": "與 app/gitea-client.js:66 同類:受信任目標、已有逾時與 MAX_PAGES 約束,風險低;正確修法需串流讀取並設上限,屬較大改動,保留待人工評估。" }, { "level": "warning", "role": "Assassin", "location": "app/gitea-client.js:106", - "problem": "將 items 直接放入 all 陣列。若 API 返回異常巨大的 JSON 陣列，可能導致容器記憶體耗盡（DoS）。(註: 亦包含 Bard 關於 for...of/push 的效能建議)", - "suggestion": "考慮在 fetchAllPages 中增加最大總項目數量的限制，並在超過時拋出錯誤；同時建議使用 AsyncGenerator 進行串流式處理以降低記憶體佔用。" - }, - { - "level": "warning", - "role": "Assassin", - "location": "app/releases.js:64", - "problem": "雖然有 encodeURIComponent，但直接將 id 拼接至 URL 是危險操作。若 id 未經妥善驗證，攻擊者可能試圖透過特殊字元擾亂 API 路徑。", - "suggestion": "建議確保 id 在進入此函數前，已驗證為預期的整數類型或符合嚴格格式的字串，不要完全依賴 encodeURIComponent 來防範所有注入可能。" - }, - { - "level": "warning", - "role": "Assassin", - "location": "app/tags.js:62", - "problem": "直接將 tag 名稱拼接至 URL 是常見的 Injection 破口。儘管使用了 encodeURIComponent，但若 tag.name 包含某些在 Gitea API 邏輯中具特殊意義的字元，仍可能造成非預期的資源存取或路徑穿越。", - "suggestion": "除了 encodeURIComponent 外，應在 config.js 或 tags.js 中對 tag.name 進行嚴格的白名單格式驗證（例如限制為英數字、點、破折號，並禁止 .. 或 /），這比單純編碼更安全。" - }, - { - "level": "warning", - "role": "Bard", - "location": "app/tags.js:19", - "problem": "函式 `categorizeTags` 內部的判斷邏輯稍微複雜，且直接在 map 內部處理多種條件分支。", - "suggestion": "建議將邏輯拆分為更小的判斷函式，提高可讀性。", - "is_new": true + "problem": "將 items 直接放入 all 陣列,若 API 回傳異常巨大的 JSON 陣列,可能導致容器記憶體耗盡(DoS)。", + "suggestion": "增加最大總項目數限制,並考慮以 AsyncGenerator 串流式處理降低記憶體佔用。", + "status": "deferred", + "defer_reason": "總量已受 MAX_PAGES(1000 頁)間接約束;改為 AsyncGenerator 串流處理屬較大架構改動,對 release/tag 數量有限的清理任務效益不高,保留待人工評估。" }, { "level": "warning", "role": "Mage", "location": "app/tags.js:46", - "problem": "在 cleanupOrphanTags 中，呼叫了兩次 API 分別獲取 releases 和 tags。若 API 在這兩次呼叫間發生變更（例如新的 release 剛好把某個舊 tag 關聯起來），categorizeTags 的結果可能會基於不一致的狀態，造成誤刪。", - "suggestion": "這是一個典型的併發/時序問題。雖然 API 本身無交易機制，但建議在取得 releases 後，若 tags 數量巨大，應考慮是否存在原子性操作的需求，或者至少在日誌中明確標示兩次獲取資料的時間間距，以便除錯。" - }, - { - "level": "warning", - "role": "Maya", - "location": "app/releases.js:46", - "problem": "在 cleanupReleases 中，若 client.fetchAllPages 拋出錯誤，流程會直接中斷且 cleanupOrphanTags 將不會被執行。雖然這符合嚴格的錯誤處理，但缺乏「部分失敗」後的清理與回報機制。", - "suggestion": "考慮在 cleanupReleases 中加入 try...catch，若僅為該步驟失敗，記錄錯誤後仍嘗試執行 cleanupOrphanTags 或明確標示整個 Action 處於部分清理狀態。至少應確保在清理失敗時，日誌能明確指出是哪一步驟導致中斷。" + "problem": "cleanupOrphanTags 分兩次 API 呼叫取得 releases 與 tags,兩次之間若 Gitea 狀態變更,categorizeTags 可能基於不一致狀態而誤刪。", + "suggestion": "考量原子性需求,或至少在日誌標示兩次取得資料的時間間距以利除錯。", + "status": "deferred", + "defer_reason": "Gitea API 無交易機制;現行已在 cleanupOrphanTags 開頭重新抓取最新 release 作為主要緩解,殘餘競態窗極小且屬排程任務可接受範圍。是否再加每筆刪除前複查屬一致性/成本取捨,保留待人工評估。" }, { "level": "warning", "role": "Maya", "location": "app/gitea-client.js:115", - "problem": "deleteResource 僅檢查狀態碼，若刪除失敗（非 204），目前實作僅回傳狀態碼，呼叫方需要處理後續的邏輯。但在測試中，對於非 204 的處理顯得較為鬆散。", - "suggestion": "建議在 deleteResource 內部就針對常見的錯誤狀態碼（如 401/403/404）進行特定的錯誤處理或拋出更有意義的例外，讓呼叫方能針對不同刪除失敗的原因採取對應策略（如：跳過、重試或完全中止）。" - }, - { - "level": "info", - "role": "Maya", - "location": "app/config.js:34", - "problem": "雖然有 requireUrl 驗證，但在 loadConfig 中，對於 GITEA_SERVER_URL 的解析假設其為完整路徑。若環境變數提供的網址不含 api/v1 或路徑有變化，整合後的 releaseApiUrl 可能無效。", - "suggestion": "建議在 loadConfig 中增加對 serverUrl 的處理，確保結尾沒有多餘的 /，避免拼接 API 路徑時產生類似 //api 的錯誤路徑。" + "problem": "deleteResource 僅回傳狀態碼,對於 401/403/404 等錯誤未做區分處理,呼叫方策略較鬆散。", + "suggestion": "在 deleteResource 內針對常見錯誤碼拋出更有意義的例外,讓呼叫方可採取跳過/重試/中止等對應策略。", + "status": "deferred", + "defer_reason": "現行刻意讓 deleteResource 單純回傳狀態碼、由呼叫端記錄 HTTP code 後續行;依狀態碼採取不同策略(如遇 401 中止)與重試/閾值同屬錯誤處理設計取捨,與 app/releases.js:38 一併保留待人工評估。" } ]