重新整理 cleanup-release 動作與文件 #1

Merged
admin merged 18 commits from ai-review-resolve/develop-20260711-131608 into develop 2026-07-15 02:38:32 +00:00
Showing only changes of commit 29b3e37b79 - Show all commits
+170
View File
@@ -0,0 +1,170 @@
[
{
"level": "critical",
"role": "Assassin",
"location": "src/index.js:72",
"problem": "這裡直接依 `GITEA_SERVER_URL` 的協定選用 `http`/`https`,沒有強制 HTTPS。只要目標位址是 `http://``Authorization: token ...` 就會明文送出,攔截者可以直接竊走權杖,並回傳假回應誘導後續刪除錯誤的 release/tag。",
"suggestion": "強制只接受 `https://` 的 API 端點,或在明確的安全開關下才允許 `http://`;建立請求前也要驗證目標主機是否為預期的 Gitea 網域。",
"is_new": true
},
{
"level": "critical",
"role": "Mage",
"location": "src/index.js:302",
"problem": "這裡在 DELETE 回傳非 204 時只寫錯誤訊息,沒有把失敗往上拋或標記成整體失敗;同樣的寫法在後面的 tag 刪除區塊也出現一次。最小重現:只要某個 release 因權限不足回 403,step 仍會繼續跑完並以成功結束,外層 workflow 會誤判清理已完成。",
"suggestion": "把刪除結果納入整體失敗狀態,例如遇到非 204 直接 `throw`,或累積 `hadFailure` 後在流程結束時 `process.exit(1)`,不要讓任何刪除失敗被靜默吞掉。",
"is_new": true
},
{
"level": "warning",
"role": "Assassin",
"location": "src/index.js:88",
"problem": "遠端回應內容被直接拼進例外訊息;一旦 API 回傳內部錯誤、堆疊或控制字元,這些內容會原封不動進入 stderr,造成資訊外洩與 log forging。",
"suggestion": "錯誤訊息只保留必要的狀態碼與簡短代碼,response body 要截斷、過濾控制字元,或乾脆不要回吐 body。",
"is_new": true
},
{
"level": "warning",
"role": "Assassin",
"location": "src/index.js:139",
"problem": "`releaseTag` 與 `releaseName` 來自遠端 API,卻未做任何跳脫就寫入 log。攻擊者若能建立包含換行或 ANSI escape 的 release 名稱,就能偽造成功/失敗紀錄,掩蓋真正的刪除行為。",
"suggestion": "記錄前先移除控制字元或改成結構化輸出,例如 JSON;不要把未信任字串直接串進 log。",
"is_new": true
},
{
"level": "warning",
"role": "Assassin",
"location": "src/index.js:164",
"problem": "`tagName` 同樣來自遠端 API,直接輸出到 log 會讓惡意 tag 名稱注入假訊息或控制終端畫面。攻擊者只要能建立特製 tag,就能污染審計紀錄。",
"suggestion": "對 tag 名稱做輸出編碼或控制字元過濾,並優先使用結構化日誌,避免未信任字串直接影響 log 內容。",
"is_new": true
},
{
"level": "warning",
"role": "Bard",
"location": "action.yml:18",
"problem": "`GitHub Runner Token` 跟整份 action 的 Gitea 語境不一致,品牌詞突然換邊,讀起來會有明顯跳拍。",
"suggestion": "改成中性的 `Runner Token` 或直接寫 `Gitea Runner Token`,保持用語一致。",
"is_new": true
},
{
"level": "warning",
"role": "Leo",
"location": "entrypoint.sh:10",
"problem": "更新時間被硬編碼在腳本裡,代表每次發布都要人工同步這個值。這種裝飾性資訊一旦和實際版本脫節,未來排查問題時反而會誤導維護者。",
"suggestion": "移除手寫時間戳,或改成由建置流程注入單一來源的版本資訊,避免多處手動更新。",
"is_new": true
},
{
"level": "warning",
"role": "Bard",
"location": "src/index.js:95",
"problem": "`success()` 這個名稱暗示它會輸出成功層級,但實際上卻跟 `info()` 一樣寫 `INF`;命名與輸出不對拍,後面看 log 的人很容易被誤導。",
"suggestion": "要嘛改成真正的成功層級代號,要嘛直接把函式命名收斂成 `info()`。",
"is_new": true
},
{
"level": "warning",
"role": "Leo",
"location": "src/index.js:212",
"problem": "這裡直接 `JSON.parse` 回應內容,沒有包一層具體的錯誤脈絡。只要 API 回傳格式稍微異常,維護者就只會拿到模糊的 syntax error,得重新重現才能知道是哪些 endpoint 出問題。",
"suggestion": "替解析失敗補上更具體的錯誤訊息,至少把 URL 和原始回應片段納入例外,讓除錯時能直接定位是哪一頁資料壞掉。",
"is_new": true
},
{
"level": "warning",
"role": "Mage",
"location": "src/index.js:312",
"problem": "這裡先抓一份 release 快照,再在後面依這份快照去刪 tag;兩個步驟之間不是同一個時間點。最小重現:cleanup 跑到一半時剛好有人新增 release,新 release 的 tag 來不及出現在 `releaseTags`,接下來的 tag 清理就可能把剛發布的 tag 誤刪。",
"suggestion": "把 release 與 tag 的判定建立在同一個一致性快照上,或在刪 tag 前重新驗證該 tag 目前是否已被任何 release 使用;如果環境允許,最好加上流程鎖避免與發版同時執行。",
"is_new": true
},
{
"level": "warning",
"role": "Maya",
"location": "src/index.js:115",
"problem": "`KEEP_COUNT` 的整數邊界現在只靠正則檢查,但沒有測試證明 `0`、`01`、負數、浮點數、非數字字串都會被正確處理。這個值直接影響刪除範圍,少一個邊界案例就可能誤刪 release。",
"suggestion": "為 `requireInteger` 與 `KEEP_COUNT` 加測試,至少覆蓋 `0`、`1`、`-1`、`1.5`、`abc`,並確認不合法輸入會退出,合法輸入會順利進入後續流程。",
"is_new": true
},
{
"level": "warning",
"role": "Maya",
"location": "src/index.js:151",
"problem": "`fetchAllPages` 新增了分頁、HTTP 狀態碼檢查、JSON 陣列驗證與空頁終止,但沒有看到對這些分支的測試。這是核心資料取得邏輯,若分頁終止條件或錯誤處理出問題,後面的刪除流程就會建立在錯誤資料上。",
"suggestion": "補測 `fetchAllPages`:成功串接多頁資料、遇到空頁停止、非 2xx 回應拋錯、回傳非陣列 JSON 拋錯。建議用 stub/mock HTTP server 驗證回傳資料與例外訊息。",
"is_new": true
},
{
"level": "warning",
"role": "Maya",
"location": "src/index.js:211",
"problem": "release 清理流程的排序與切片邏輯現在直接決定會刪掉哪些項目,但沒有測試保證 `created_at` 是由新到舊排序後再依 `KEEP_COUNT` 保留。只要排序方向或切片位置錯一格,就會變成刪掉最新的 release。",
"suggestion": "新增 release 清理的整合測試,輸入刻意亂序的 `created_at` 資料,驗證只保留最新 `KEEP_COUNT` 筆;再補上 `KEEP_COUNT=0`、`KEEP_COUNT=releaseCount` 與 `releaseItem.id` 缺失時會略過刪除的案例。",
"is_new": true
},
{
"level": "warning",
"role": "Maya",
"location": "src/index.js:269",
"problem": "tag 清理流程新增了『保留已對應 release 的 tag』、『刪除未指定 release 的 tag』、以及無名稱 tag 略過與刪除失敗處理,但目前看不到任何對應測試。這條路徑如果誤刪 tag,會直接破壞版本辨識。",
"suggestion": "補測 tag 清理行為:對應 release 的 tag 必須保留、未被任何 release 引用的 tag 必須被刪除、空名稱 tag 必須略過,並驗證 DELETE 非 204 時會走錯誤分支。",
"is_new": true
},
{
"level": "warning",
"role": "Rogue",
"location": "src/index.js:12",
"problem": "`formatTaipeiTimestamp()` 每次 log 都重新建立 `Intl.DateTimeFormat` 並跑 `formatToParts`,這在 release/tag 迴圈裡會被反覆觸發,等於把本來可重用的格式器成本重算 N 次。",
"suggestion": "把 `Intl.DateTimeFormat` 提到函式外快取成單例,讓每次只做時間格式化,不要重建 formatter。",
"is_new": true
},
{
"level": "warning",
"role": "Rogue",
"location": "src/index.js:312",
"problem": "這裡又對 releases API 做一次完整 `fetchAllPages()`,前面第 267 行已經抓過同一份資料並排序;等於把整個分頁抓取、JSON 解析與記憶體配置再跑一遍,資料量越大越浪費。",
"suggestion": "直接沿用前一次抓到的 `releaseJson`,或先從第一次結果算出要保留的 tag 集合,避免第二次全量拉取。",
"is_new": true
},
{
"level": "warning",
"role": "Rogue",
"location": "src/index.js:290",
"problem": "這個 `for` 迴圈把每個 release 的 DELETE 都串成單一等待鏈;如果要刪的 release 有 N 筆,就會多吃 N 次網路往返,整體牆鐘時間被 RTT 線性放大。",
"suggestion": "如果 Gitea API 容許,改成有限度並行刪除,例如一次 4 到 8 筆,或至少把可獨立的請求批次化。",
"is_new": true
},
{
"level": "warning",
"role": "Rogue",
"location": "src/index.js:326",
"problem": "tag 刪除同樣是逐筆 `await`,當 tag 數量多時會把每次 API 往返都串成排隊,刪除時間幾乎全卡在網路延遲上。",
"suggestion": "用受限並行處理 tag 刪除,或先收集待刪清單再批次送出,減少總等待時間。",
"is_new": true
},
{
"level": "info",
"role": "Bard",
"location": "src/index.js:69",
"problem": "空的 `separator()` 函式只是佔位,既不做事也不自我說明,還讓檔案多了一個無效符號。",
"suggestion": "移除這個空函式,或改成真正有用途的共用輸出 helper。",
"is_new": true
},
{
"level": "info",
"role": "Bard",
"location": "src/index.js:162",
"problem": "`requestJson` 這個名字太窄,因為它不只用來拿 JSON,也拿一般 HTTP 回應與 DELETE 結果;名稱比實作更嚴格,讀者會先被騙一次。",
"suggestion": "改名成 `request`、`requestUrl` 之類較中性的名稱,JSON 解析再交給上層 helper。",
"is_new": true
},
{
"level": "info",
"role": "Leo",
"location": "Dockerfile:5",
"problem": "基底映像預設成 `alpine` 這種浮動標籤,長期看會讓建置結果跟著上游變動。半年後同一份程式碼可能產生不同映像,維護者很難判斷差異到底來自程式還是基底環境。",
"suggestion": "把預設值改成明確版本或 digest,讓基底環境可預期;如果要保留可變版本,至少把它明確視為建置參數而不是默認行為。",
"is_new": true
}
]