chore(ai-review): 更新 findings.json 與 exclusions.json
CI / AI Code Review (pull_request) Failing after 50s
CI / AI Code Review (pull_request) Failing after 50s
移除本輪已修復(server URL 正規化、id 整數驗證、整合測試)與重複提報的 finding; 保留 7 條設計取捨(重試/錯誤處理/DoS/TOCTOU/deleteResource 策略); 新增 3 條不適用至 exclusions(baseUrl 由已驗證 config 組成、tag 白名單非必要、 categorizeTags 拆分無益)。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
edc933be73
commit
7b742ed85e
@@ -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,語意已清楚且具完整測試;為三個簡單條件再抽出微函式只會增加跳轉與閱讀成本,可讀性無實質提升。"
|
||||
}
|
||||
]
|
||||
|
||||
@@ -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 一併保留待人工評估。"
|
||||
}
|
||||
]
|
||||
|
||||
Reference in New Issue
Block a user