chore: update ai-review findings [ai-review-bot][failure]
This commit is contained in:
@@ -1,4 +1,18 @@
|
|||||||
[
|
[
|
||||||
|
{
|
||||||
|
"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",
|
"level": "warning",
|
||||||
"role": "Mage",
|
"role": "Mage",
|
||||||
@@ -6,16 +20,8 @@
|
|||||||
"problem": "清理流程對網路請求依賴強,若 API 呼叫失敗,整個 main 流程中斷,無法確保後續清理的一致性與部分成功重試。",
|
"problem": "清理流程對網路請求依賴強,若 API 呼叫失敗,整個 main 流程中斷,無法確保後續清理的一致性與部分成功重試。",
|
||||||
"suggestion": "引入更細緻的錯誤處理(如錯誤閾值機制)或部分清理成功後的重試策略。",
|
"suggestion": "引入更細緻的錯誤處理(如錯誤閾值機制)或部分清理成功後的重試策略。",
|
||||||
"status": "deferred",
|
"status": "deferred",
|
||||||
"defer_reason": "錯誤處理/重試策略的設計取捨。目前單筆刪除失敗會記錄並繼續、讀取失敗則中止屬合理保守行為;是否引入閾值/重試保留待人工評估。"
|
"defer_reason": "錯誤處理/重試策略的設計取捨。目前單筆刪除失敗會記錄並繼續、讀取失敗則中止屬合理保守行為;是否引入閾值/重試保留待人工評估。",
|
||||||
},
|
"is_new": false
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Leo",
|
|
||||||
"location": "app/releases.js:37",
|
|
||||||
"problem": "cleanupReleases 同時處理資料獲取、邏輯判斷與副作用執行,違反單一職責原則。",
|
|
||||||
"suggestion": "拆分邏輯與執行層,將刪除副作用抽象化為獨立函式。",
|
|
||||||
"status": "deferred",
|
|
||||||
"defer_reason": "純邏輯(selectReleasesToDelete)已抽離且函式短小、已具失敗路徑/編碼/stderr/決定性排序測試;進一步拆出刪除迴圈為設計偏好,效益有限,保留待人工評估。"
|
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "warning",
|
"level": "warning",
|
||||||
@@ -24,7 +30,8 @@
|
|||||||
"problem": "錯誤路徑以 res.text() 讀取整個回應主體,未限制大小,惡意伺服器可回傳極大內容導致記憶體耗盡(DoS)。",
|
"problem": "錯誤路徑以 res.text() 讀取整個回應主體,未限制大小,惡意伺服器可回傳極大內容導致記憶體耗盡(DoS)。",
|
||||||
"suggestion": "限制讀取的回應大小,例如檢查 Content-Length 或以串流方式設定讀取上限。",
|
"suggestion": "限制讀取的回應大小,例如檢查 Content-Length 或以串流方式設定讀取上限。",
|
||||||
"status": "deferred",
|
"status": "deferred",
|
||||||
"defer_reason": "風險低:目標為已通過 URL 驗證的受信任 Gitea 實例,且每請求已有 30 秒逾時。正確修法需串流逐段讀取並設位元組上限,屬較大改動,保留待人工評估。"
|
"defer_reason": "風險低:目標為已通過 URL 驗證的受信任 Gitea 實例,且每請求已有 30 秒逾時。正確修法需串流逐段讀取並設位元組上限,屬較大改動,保留待人工評估。",
|
||||||
|
"is_new": false
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "warning",
|
"level": "warning",
|
||||||
@@ -33,33 +40,64 @@
|
|||||||
"problem": "成功路徑以 res.json() 解析整個回應,未限制大小,惡意伺服器可回傳極大 JSON 導致記憶體耗盡(DoS)。",
|
"problem": "成功路徑以 res.json() 解析整個回應,未限制大小,惡意伺服器可回傳極大 JSON 導致記憶體耗盡(DoS)。",
|
||||||
"suggestion": "對 API 回應設定明確大小上限,超過時拒絕解析並拋出異常。",
|
"suggestion": "對 API 回應設定明確大小上限,超過時拒絕解析並拋出異常。",
|
||||||
"status": "deferred",
|
"status": "deferred",
|
||||||
"defer_reason": "與 app/gitea-client.js:66 同類:受信任目標、已有逾時與 MAX_PAGES 約束,風險低;正確修法需串流讀取並設上限,屬較大改動,保留待人工評估。"
|
"defer_reason": "與 app/gitea-client.js:66 同類:受信任目標、已有逾時與 MAX_PAGES 約束,風險低;正確修法需串流讀取並設上限,屬較大改動,保留待人工評估。",
|
||||||
},
|
"is_new": false
|
||||||
{
|
|
||||||
"level": "warning",
|
|
||||||
"role": "Assassin",
|
|
||||||
"location": "app/config.js:34",
|
|
||||||
"problem": "將環境變數值直接輸出至日誌,若內容含惡意字元或過長,可能造成 Log Injection 或日誌資源耗盡。",
|
|
||||||
"suggestion": "輸出前進行 sanitization,移除控制字元並限制長度,類似 gitea-client.js 的 sanitizeBody。",
|
|
||||||
"status": "deferred",
|
|
||||||
"defer_reason": "風險低:GITEA_SERVER_URL/GITEA_REPOSITORY/KEEP_COUNT 來自 CI 平台提供的 gitea context,並由 requireUrl/requireRepository/requireInteger 限定為不含控制字元的格式;log 注入需攻擊者控制 CI context 值才成立。可日後將記錄移至驗證後或加 sanitize,保留待人工評估。"
|
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "warning",
|
"level": "warning",
|
||||||
"role": "Assassin",
|
"role": "Assassin",
|
||||||
"location": "app/gitea-client.js:106",
|
"location": "app/gitea-client.js:106",
|
||||||
"problem": "將 items 直接放入 all 陣列,若 API 回傳異常巨大的 JSON 陣列,可能導致容器記憶體耗盡(DoS)。",
|
"problem": "將 items 直接放入 all 陣列。若 API 返回異常巨大的 JSON 陣列,可能導致容器記憶體耗盡(DoS)。(註: 亦包含 Bard 關於 for...of/push 的效能建議)",
|
||||||
"suggestion": "增加最大總項目數限制,並考慮以 AsyncGenerator 串流式處理降低記憶體佔用。",
|
"suggestion": "考慮在 fetchAllPages 中增加最大總項目數量的限制,並在超過時拋出錯誤;同時建議使用 AsyncGenerator 進行串流式處理以降低記憶體佔用。"
|
||||||
"status": "deferred",
|
},
|
||||||
"defer_reason": "總量已受 MAX_PAGES(1000 頁)間接約束;改為 AsyncGenerator 串流處理屬較大架構改動,對 release/tag 數量有限的清理任務效益不高,保留待人工評估。"
|
{
|
||||||
|
"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
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"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 處於部分清理狀態。至少應確保在清理失敗時,日誌能明確指出是哪一步驟導致中斷。"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"level": "warning",
|
||||||
|
"role": "Maya",
|
||||||
|
"location": "app/gitea-client.js:115",
|
||||||
|
"problem": "deleteResource 僅檢查狀態碼,若刪除失敗(非 204),目前實作僅回傳狀態碼,呼叫方需要處理後續的邏輯。但在測試中,對於非 204 的處理顯得較為鬆散。",
|
||||||
|
"suggestion": "建議在 deleteResource 內部就針對常見的錯誤狀態碼(如 401/403/404)進行特定的錯誤處理或拋出更有意義的例外,讓呼叫方能針對不同刪除失敗的原因採取對應策略(如:跳過、重試或完全中止)。"
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"level": "info",
|
"level": "info",
|
||||||
"role": "Bard",
|
"role": "Maya",
|
||||||
"location": "app/releases.js:28",
|
"location": "app/config.js:34",
|
||||||
"problem": "config 參數型別 ReturnType<import('./config.js').loadConfig> 雖準確但冗長,影響可讀性。",
|
"problem": "雖然有 requireUrl 驗證,但在 loadConfig 中,對於 GITEA_SERVER_URL 的解析假設其為完整路徑。若環境變數提供的網址不含 api/v1 或路徑有變化,整合後的 releaseApiUrl 可能無效。",
|
||||||
"suggestion": "在 config.js 定義並匯出 JSDoc @typedef,於各處改用該別名。",
|
"suggestion": "建議在 loadConfig 中增加對 serverUrl 的處理,確保結尾沒有多餘的 /,避免拼接 API 路徑時產生類似 //api 的錯誤路徑。"
|
||||||
"status": "deferred",
|
|
||||||
"defer_reason": "純可讀性偏好,現有型別註解準確且可運作;引入 @typedef 會牽動多處 JSDoc 與 README 行號,效益有限,保留待人工評估。"
|
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
|
|||||||
Reference in New Issue
Block a user