From b461f72d4368b8baa038b7694f16aba3202638be Mon Sep 17 00:00:00 2001 From: Jeffery Date: Fri, 26 Jun 2026 11:43:16 +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 移除已修復(決定性排序)與多項重複提報的 finding;保留 7 條設計取捨; 新增 6 條不適用/誤報至 exclusions(內部 IP 黑名單會破壞自架 Gitea、 section 排版主觀、無效日期已註記、CI 堆疊輸出無妨、MAX_PAGES 為安全上限、 通用 log 抽象效益低)。 Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitea/ai-review/exclusions.json | 36 ++++++++++ .gitea/ai-review/findings.json | 119 +++++-------------------------- 2 files changed, 52 insertions(+), 103 deletions(-) diff --git a/.gitea/ai-review/exclusions.json b/.gitea/ai-review/exclusions.json index 5b5192d..ae4eff0 100644 --- a/.gitea/ai-review/exclusions.json +++ b/.gitea/ai-review/exclusions.json @@ -70,5 +70,41 @@ "role": "Rogue", "original_finding": "為排序而 map 成物件陣列再 map 回原物件,造成多次 O(n) 陣列分配與 GC 壓力,建議減少中間狀態或原地排序。", "reason": "不採納且與前次建議衝突。此 map/sort/map(Schwartzian transform)正是前一輪 Rogue(releases.js:17)要求「排序前預先計算時間戳」的實作,用以避免比較器重複呼叫 Date.parse;release 數量有限,中間陣列開銷可忽略,可讀性更佳。" + }, + { + "location": "app/config.js:35", + "role": "Assassin", + "original_finding": "requireUrl 僅驗證 URL 格式,未阻擋指向內部網路/loopback(localhost、169.254.169.254 等)的位址,容器環境恐 SSRF。建議加入內部 IP 黑名單。", + "reason": "不適用且會破壞正常部署。GITEA_SERVER_URL 來自 CI 平台提供的 gitea.server_url,即本 Action 欲操作的 Gitea 實例本身;自架 Gitea 經常部署於內網/私有位址,封鎖內部 IP 會使合法部署無法運作。此非攻擊者可控的任意外連目標。" + }, + { + "location": "app/releases.js:46", + "role": "Bard", + "original_finding": "區段標題層級不一致:『取得成品資訊』內含 info 輸出,『刪除舊版本成品』直接開始邏輯,視覺節奏稍不一致。", + "reason": "主觀排版意見。現行 section() 用於標示每個主要階段,語意清楚且符合原 bash 版本結構;不構成功能或可讀性問題。" + }, + { + "location": "app/releases.js:20", + "role": "Leo", + "original_finding": "selectReleasesToDelete 對無法解析的日期視為 0 是隱晦行為,維護者可能不解,建議記錄 warning 或明確處理。", + "reason": "已於程式碼註解明確記錄『格式無效或缺漏時視為最舊(0)』;Gitea API 一向回傳有效 created_at,對每筆無效日期發出 runtime warning 將是不會觸發的死碼與雜訊,不採納。" + }, + { + "location": "app/logger.js:53", + "role": "Assassin", + "original_finding": "failError 直接輸出 error.stack 可能洩漏專案目錄結構、內部函式名稱等敏感路徑;建議生產環境隱藏堆疊或僅 debug 模式輸出。", + "reason": "不適用。本專案為 CI/CD 清理 Action,堆疊輸出至僅儲存庫維護者可見的 Action 日誌以利除錯,非公開環境;容器內路徑為 /app,不含敏感資訊。隱藏堆疊反而降低可維運性。" + }, + { + "location": "app/gitea-client.js:10", + "role": "Leo", + "original_finding": "MAX_PAGES 寫死於程式碼,建議改為環境變數傳入以增加部署彈性。", + "reason": "不採納。MAX_PAGES 為防止無限迴圈的安全上限(backstop),非調校參數;1000 頁遠超任何真實儲存庫的 release/tag 規模,改為環境變數只會增加不必要的設定面與誤設風險。" + }, + { + "location": "app/logger.js:46", + "role": "Leo", + "original_finding": "建議在 logger.js 建立通用 log(level, prefix, stream) 函式,讓 info/warn/fail 等共用以降低重複。", + "reason": "不採納。六個輸出函式皆為單行、語意直觀;為此抽象通用 dispatcher 反而增加間接層與閱讀成本,去重效益微小。" } ] diff --git a/.gitea/ai-review/findings.json b/.gitea/ai-review/findings.json index 7c766ec..6f4a410 100644 --- a/.gitea/ai-review/findings.json +++ b/.gitea/ai-review/findings.json @@ -1,28 +1,4 @@ [ - { - "level": "critical", - "role": "Maya", - "problem": "測試只驗證了正常與極端的 token 輸入，但完全沒有測試 token 為 null 或未定義（匿名模式）下的處理邏輯，也沒確認匿名請求時，設定物件是否正確地將 token 設為 null。", - "suggestion": "請增加測試案例，明確斷言當 GITEA_TOKEN 不存在於環境變數時，loadConfig() 回傳的 token 屬性為 null。", - "location": "app/test/config.test.js:63", - "is_new": false - }, - { - "level": "critical", - "role": "Maya", - "problem": "測試中雖然有 fetchAllPages 的功能測試，但對於 MAX_PAGES 的邊界情況，測試只用了 globalThis.fetch 永遠回傳非空陣列，這是一個快樂路徑的極端變體。如果 API 剛好在第 MAX_PAGES 頁回傳空陣列，測試並未驗證客戶端是否能正確處理並停止。", - "suggestion": "請增加一個測試案例，模擬當 API 恰好在第 MAX_PAGES 次請求時回傳空陣列的情境，確認客戶端能成功結束，而非拋出例外。", - "location": "app/test/gitea-client.test.js:33", - "is_new": false - }, - { - "level": "critical", - "role": "Assassin", - "location": "app/config.js:35", - "problem": "僅驗證是否為 URL 格式，未檢查傳入的 `GITEA_SERVER_URL` 是否指向內部網路敏感資源（如 localhost, 169.254.169.254 等），這在容器化環境中可能導致 SSRF（伺服器端請求偽造）。", - "suggestion": "在 `validate.js` 的 `requireUrl` 中加入黑名單機制，禁止解析為內部 IP 位址或 loopback 位址。", - "is_new": true - }, { "level": "warning", "role": "Mage", @@ -30,8 +6,7 @@ "problem": "清理流程對網路請求依賴強,若 API 呼叫失敗,整個 main 流程中斷,無法確保後續清理的一致性與部分成功重試。", "suggestion": "引入更細緻的錯誤處理(如錯誤閾值機制)或部分清理成功後的重試策略。", "status": "deferred", - "defer_reason": "錯誤處理/重試策略的設計取捨。目前單筆刪除失敗會記錄並繼續、讀取失敗則中止屬合理保守行為;是否引入閾值/重試保留待人工評估。", - "is_new": false + "defer_reason": "錯誤處理/重試策略的設計取捨。目前單筆刪除失敗會記錄並繼續、讀取失敗則中止屬合理保守行為;是否引入閾值/重試保留待人工評估。" }, { "level": "warning", @@ -40,8 +15,7 @@ "problem": "cleanupReleases 同時處理資料獲取、邏輯判斷與副作用執行,違反單一職責原則。", "suggestion": "拆分邏輯與執行層,將刪除副作用抽象化為獨立函式。", "status": "deferred", - "defer_reason": "純邏輯(selectReleasesToDelete)已抽離且函式短小、已具失敗路徑/編碼/stderr 測試;進一步拆出刪除迴圈為設計偏好,效益有限,保留待人工評估。", - "is_new": false + "defer_reason": "純邏輯(selectReleasesToDelete)已抽離且函式短小、已具失敗路徑/編碼/stderr/決定性排序測試;進一步拆出刪除迴圈為設計偏好,效益有限,保留待人工評估。" }, { "level": "warning", @@ -50,8 +24,7 @@ "problem": "錯誤路徑以 res.text() 讀取整個回應主體,未限制大小,惡意伺服器可回傳極大內容導致記憶體耗盡(DoS)。", "suggestion": "限制讀取的回應大小,例如檢查 Content-Length 或以串流方式設定讀取上限。", "status": "deferred", - "defer_reason": "風險低:目標為已通過 URL 驗證的受信任 Gitea 實例,需伺服器被入侵或中間人攻擊才成立,且每請求已有 30 秒逾時部分約束。正確修法需串流逐段讀取並設位元組上限(Content-Length 在 chunked 下不可靠),屬較大改動,保留待人工評估。", - "is_new": false + "defer_reason": "風險低:目標為已通過 URL 驗證的受信任 Gitea 實例,且每請求已有 30 秒逾時。正確修法需串流逐段讀取並設位元組上限,屬較大改動,保留待人工評估。" }, { "level": "warning", @@ -60,93 +33,33 @@ "problem": "成功路徑以 res.json() 解析整個回應,未限制大小,惡意伺服器可回傳極大 JSON 導致記憶體耗盡(DoS)。", "suggestion": "對 API 回應設定明確大小上限,超過時拒絕解析並拋出異常。", "status": "deferred", - "defer_reason": "與 app/gitea-client.js:66 同類:受信任目標、已有逾時與 MAX_PAGES 約束,風險低;正確修法需串流讀取並設上限,屬較大改動,保留待人工評估。", - "is_new": false - }, - { - "level": "warning", - "role": "Maya", - "problem": "在 `cleanupReleases` 的整合測試中，雖然有測試 `deleteResource` 回傳非 204 時的行為，但測試只檢查了 `client.deleted` 的呼叫順序，並沒有驗證 `fail` 日誌（對 stderr 的寫入）是否正確被觸發。", - "suggestion": "建議攔截 `process.stderr.write`，確認當 `deleteResource` 回傳 500 時，系統確實有記錄到錯誤訊息。", - "location": "app/test/releases.test.js:77", - "is_new": false - }, - { - "level": "warning", - "role": "Maya", - "problem": "在 `cleanupOrphanTags` 的測試中，缺乏對於「當 `deleteResource` 失敗」時的行為驗證。目前只測試了成功的情境。", - "suggestion": "增加模擬 `deleteResource` 回傳非 204 狀態碼的情境，確認該項刪除失敗不會中斷整個標籤清理流程，並檢查是否有對應的錯誤日誌。", - "location": "app/test/tags.test.js:73", - "is_new": false + "defer_reason": "與 app/gitea-client.js:66 同類:受信任目標、已有逾時與 MAX_PAGES 約束,風險低;正確修法需串流讀取並設上限,屬較大改動,保留待人工評估。" }, { "level": "warning", "role": "Assassin", "location": "app/config.js:34", - "problem": "將環境變數值直接輸出至日誌，若 `GITEA_SERVER_URL` 等變數內容被注入惡意字元或過長，可能造成 Log Injection 或日誌系統資源耗盡。", - "suggestion": "在輸出前進行 sanitization，移除控制字元並限制長度，類似於 `gitea-client.js` 中的 `sanitizeBody`。", - "is_new": true + "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", "role": "Assassin", "location": "app/gitea-client.js:106", - "problem": "將 `items` 直接放入 `all` 陣列。若 API 返回異常巨大的 JSON 陣列，可能導致容器記憶體耗盡（DoS）。(註: 亦包含 Bard 關於 for...of/push 的效能建議)", - "suggestion": "考慮在 `fetchAllPages` 中增加最大總項目數量的限制，並在超過時拋出錯誤；同時建議使用 `AsyncGenerator` 進行串流式處理以降低記憶體佔用。" - }, - { - "level": "warning", - "role": "Bard", - "location": "app/releases.js:46", - "problem": "區段標題『取得成品資訊』與後續的 `section('刪除舊版本成品')` 使用了不同的區段層級。第一個區段內包含了 `info` 輸出，而第二個區段直接開始處理邏輯，視覺節奏上稍微不一致。", - "suggestion": "建議在所有主要的操作階段前統一呼叫 `section()`，或在細部操作前使用更明確的層級標示，保持日誌格式的旋律一致性。", - "is_new": true - }, - { - "level": "warning", - "role": "Leo", - "location": "app/releases.js:20", - "problem": "在 `selectReleasesToDelete` 函式中，對於 `Date.parse` 無法解析的日期直接視為 `0`，這在資料清理邏輯中是一個隱晦的行為，未來維護者可能不清楚為什麼無效日期會優先被刪除。", - "suggestion": "應明確記錄無效日期的處理方式（例如記錄 warning），或在 `Date.parse` 失敗時，應考慮給予一個明確的邏輯（例如拋出錯誤或放到特定排序位置），以減少不可預期的副作用。", - "is_new": true - }, - { - "level": "warning", - "role": "Mage", - "location": "app/releases.js:23", - "problem": "在 `selectReleasesToDelete` 的排序邏輯中，若多個成品具有完全相同的 `created_at` 時間戳，目前的排序行為依賴於 JavaScript 引擎對 `sort()` 的實作（在某些情況下可能不穩定），導致保留與刪除的成品選擇具有不確定性。", - "suggestion": "建議在排序邏輯中加入次要的排序鍵值（如 `id` 或 `tag_name`）作為比較依據（例如：若時間相同，則比較 ID 大小），以確保排序結果在時間相同時仍具有決定性。", - "is_new": true + "problem": "將 items 直接放入 all 陣列,若 API 回傳異常巨大的 JSON 陣列,可能導致容器記憶體耗盡(DoS)。", + "suggestion": "增加最大總項目數限制,並考慮以 AsyncGenerator 串流式處理降低記憶體佔用。", + "status": "deferred", + "defer_reason": "總量已受 MAX_PAGES(1000 頁)間接約束;改為 AsyncGenerator 串流處理屬較大架構改動,對 release/tag 數量有限的清理任務效益不高,保留待人工評估。" }, { "level": "info", "role": "Bard", "location": "app/releases.js:28", - "problem": "config 參數型別定義方式雖然準確，但過於冗長且與實作細節耦合過深，影響程式碼的可讀性與簡潔度。", - "suggestion": "建議在 `app/config.js` 定義並匯出型別註解（JSDoc @typedef），然後在各處直接使用該別名。" - }, - { - "level": "info", - "role": "Assassin", - "location": "app/logger.js:53", - "problem": "儘管使用了 `stderr` 輸出錯誤，但在 `failError` 中直接輸出 `error.stack` 可能會洩漏專案目錄結構、內部函式名稱等敏感路徑資訊。", - "suggestion": "在生產環境下考慮隱藏堆疊追蹤，或僅在特定 debug 模式下輸出堆疊。", - "is_new": true - }, - { - "level": "info", - "role": "Leo", - "location": "app/gitea-client.js:10", - "problem": "目前 `MAX_PAGES` 是寫死在程式碼中的常數，未來如果 API 規格變更或是特殊儲存庫的 Release 數量激增，維護者需要進程式碼修改，且這在不同的環境下可能需要不同的上限。", - "suggestion": "建議將 `MAX_PAGES` 改為透過環境變數傳入，並設定一個合理的預設值，增加部署時的彈性。", - "is_new": true - }, - { - "level": "info", - "role": "Leo", - "location": "app/logger.js:46", - "problem": "雖然目前只有 `fail` 函式會寫入 `stderr`，但如果有更多的 error level 需要處理，分散的邏輯會增加維護成本。", - "suggestion": "考慮在 `logger.js` 中建立一個通用的 `log` 函式，處理 `level`、`prefix` 與 `stream` 的對應，讓其他方法（如 `info`, `warn`, `fail`）只負責呼叫該通用函式，降低重複程式碼。", - "is_new": true + "problem": "config 參數型別 ReturnType 雖準確但冗長,影響可讀性。", + "suggestion": "在 config.js 定義並匯出 JSDoc @typedef,於各處改用該別名。", + "status": "deferred", + "defer_reason": "純可讀性偏好,現有型別註解準確且可運作;引入 @typedef 會牽動多處 JSDoc 與 README 行號,效益有限,保留待人工評估。" } ]