From 053109662def25986caa30bd08c4be9ac0c6fe2b Mon Sep 17 00:00:00 2001 From: Jeffery Date: Wed, 24 Jun 2026 14:01:14 +0000 Subject: [PATCH] =?UTF-8?q?chore(ai-review=20=E7=8B=80=E6=85=8B):=20?= =?UTF-8?q?=E7=A7=BB=E9=99=A4=E5=B7=B2=E8=99=95=E7=90=86=20findings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gitea/ai-review/exclusions.json | 6 ++ .gitea/ai-review/findings.json | 155 +------------------------------ 2 files changed, 7 insertions(+), 154 deletions(-) diff --git a/.gitea/ai-review/exclusions.json b/.gitea/ai-review/exclusions.json index 4d85379..8e9acd5 100644 --- a/.gitea/ai-review/exclusions.json +++ b/.gitea/ai-review/exclusions.json @@ -136,5 +136,11 @@ "role": "Leo", "original_finding": "建議至少加上 `console.error` 或在開發/除錯模式下將錯誤拋出，以便在清除失敗時能收到警示。", "reason": "AI 對話收斂判定為誤報（問題在最新程式碼中不成立或不適用）" + }, + { + "location": "app/main.js:125", + "role": "Mage", + "original_finding": "這裡直接使用 `spawn` 執行 `codex` 命令，且參數 `prompt` 是直接從 `process.env.PROMPT` 讀取並傳入的。如果 CI 環境的 `PROMPT` 被惡意竄改，雖使用陣列傳遞參數避免了 shell injection，但 `codex exec` 的邏輯若沒有妥善限制（例如限制可執行指令類型），可能導致攻擊者在 CI Runner 環境執行任意指令。", + "reason": "此 action 的目的就是讓呼叫 workflow 以 `PROMPT` 指定 Codex 任務並在隔離 CI 容器內非互動執行；prompt 語意必須由 workflow 與 secret 的信任邊界控管。程式已用 `spawn` 參數陣列避免 shell injection，不適合在 action 內以白名單改寫或限制使用者 prompt。" } ] diff --git a/.gitea/ai-review/findings.json b/.gitea/ai-review/findings.json index 5060c6d..fe51488 100644 --- a/.gitea/ai-review/findings.json +++ b/.gitea/ai-review/findings.json @@ -1,154 +1 @@ -[ - { - "level": "critical", - "role": "Mage", - "problem": "在 `setupAuth` 中，`lockPath` 使用 `os.tmpdir()`。在共享環境中，如果 `CODEX_HOME` 字串相同，會導致所有 process 競爭同一個鎖檔，且如果其他無關的 process 也剛好在 `os.tmpdir()` 中建立相同名稱的檔案，會導致誤判或鎖定失敗。", - "suggestion": "應在 `CODEX_HOME` 內部建立鎖檔，而非使用全域的 `os.tmpdir()`，或者包含更具唯一性的識別碼（如 PID 或更長的路徑雜湊）以確保鎖的隔離性。", - "location": "app/main.js:189", - "is_new": false - }, - { - "level": "critical", - "role": "Mage", - "location": "app/main.js:125", - "problem": "這裡直接使用 `spawn` 執行 `codex` 命令，且參數 `prompt` 是直接從 `process.env.PROMPT` 讀取並傳入的。如果 CI 環境的 `PROMPT` 被惡意竄改，雖使用陣列傳遞參數避免了 shell injection，但 `codex exec` 的邏輯若沒有妥善限制（例如限制可執行指令類型），可能導致攻擊者在 CI Runner 環境執行任意指令。", - "suggestion": "在 `runCodex` 函式中，除了已經加入的 `--dangerously-bypass-approvals-and-sandbox` 外，必須確保對 `prompt` 進行強力的白名單過濾，或改為使用非 `exec` 的子指令來限制權限。", - "is_new": true - }, - { - "level": "critical", - "role": "Mage", - "location": "app/main.js:255", - "problem": "在 `setupAuth` 中，使用了 `fs.renameSync` 來確保原子性。然而在某些檔案系統中，若 `authFile` 與 `authPath` 不在同一個分區，`renameSync` 可能會失敗。此外，如果 `codexHome` 已存在且權限配置錯誤，`fs.mkdirSync` 可能會被忽略但後續存取失敗。", - "suggestion": "建議確保 `authFile` 與 `codexHome` 處於相同掛載點，並增加對 `fs.mkdirSync` 後權限檢查的驗證。", - "is_new": true - }, - { - "level": "warning", - "role": "Rogue", - "problem": "在 setupAuth 中，`lockPath` 檔名產生使用了 `Buffer.from(codexHome).toString(\"hex\")`。如果 `codexHome` 非常長，這個檔名可能會超過作業系統的檔案名稱長度限制（通常為 255 bytes），導致鎖定失敗，進而阻斷整個流程。", - "suggestion": "改用 `crypto.createHash('sha256').update(codexHome).digest('hex')` 來產生固定長度的雜湊值作為檔名的一部分，既安全又保證長度可控。", - "location": "app/main.js:192", - "is_new": false - }, - { - "level": "warning", - "role": "Mage", - "problem": "在 `setupAuth` 中，在 `fs.copyFileSync(authFile, authPath)` 後立即 `removeIfCreated(authFile)`，但在這期間如果發生 process 中斷，auth.json 可能會以不安全的權限（預設）或不完整的狀態寫入。", - "suggestion": "建議使用 `fs.renameSync` 或在完成寫入與權限設定後再進行清理，並確保寫入過程中發生異常時能正確刪除該部分寫入的檔案。", - "location": "app/main.js:203", - "is_new": false - }, - { - "level": "warning", - "role": "Maya", - "problem": "在執行外部指令時沒有設定逾時（timeout），若 Codex CLI 發生無預期的掛起（hang），Action 將會永久卡住而不會自動終止。", - "suggestion": "建議在 `spawn` 的選項中加入 `timeout` 機制，或是主動在啟動後設置一個計時器，當執行時間過長時強制終止子行程。", - "location": "app/main.js:102", - "is_new": false - }, - { - "level": "warning", - "role": "Mage", - "problem": "在 `runCodex` 中使用 `spawn` 時，沒有設定 `cwd`。如果 `codex` 工具依賴於當前工作目錄（例如需要編輯當前專案），這在 CI 環境中可能存在風險，雖然目前 CI 通常會設定好目錄，但這是一個隱含的契約。", - "suggestion": "建議明確設定 `cwd` 為 `/github/workspace` 或 CI 定義的專案根目錄，確保 `codex` 運作在預期的上下文中。", - "location": "app/main.js:114", - "is_new": false - }, - { - "level": "warning", - "role": "Maya", - "problem": "直接將所有輸出串接在 `outputChunks` 中，若 CLI 輸出過大的日誌，可能會導致記憶體耗盡（OOM）。", - "suggestion": "建議針對 output 大小設定上限，超過限制時截斷輸出，或是改用串流寫入暫存檔以避免將所有內容存於記憶體。", - "location": "app/main.js:125", - "is_new": false - }, - { - "level": "warning", - "role": "Mage", - "problem": "在 `validateAuth` 中，僅透過 `JSON.parse` 檢查 JSON 格式，但未針對 Codex 預期的 auth.json 結構（如必要的欄位）進行 Schema 驗證。如果傳入的 JSON 格式正確但內容無效，可能會導致 `codex exec` 在後續執行時失敗。", - "suggestion": "建議加入對 JSON 內容的簡單結構驗證（例如確認是否有 `token` 或必要的連線設定欄位）。", - "location": "app/main.js:77", - "is_new": false - }, - { - "level": "warning", - "role": "Bard", - "location": "app/main.js:120", - "problem": "validateAuth 函式中對於 Base64 的正規化與驗證邏輯混雜在一起，使用了大量的取代與判斷，讀起來節奏凌亂，缺乏優雅感。", - "suggestion": "將驗證邏輯與基礎轉換邏輯抽離，建議提取一個輔助函式專門負責 Base64 格式檢查，使主要流程清晰明瞭。", - "is_new": true - }, - { - "level": "warning", - "role": "Bard", - "location": "app/main.js:166", - "problem": "runCodex 函式過於臃腫，包含了執行、超時處理、輸出截斷與錯誤捕捉等多重責任，這段旋律太過冗長且複雜。", - "suggestion": "建議將輸出處理 (Output truncation logic) 與超時設定分離為獨立函式，以提升函式的可讀性與維護性。", - "is_new": true - }, - { - "level": "warning", - "role": "Mage", - "location": "app/main.js:77", - "problem": "在 `appendGithubOutput` 函式中，當 `output` 內容極大時，此處會將整個 `output` 字串在記憶體中進行檢查（`output.includes(delimiter)`）與多次複製。這可能導致在處理極端長度輸出時發生記憶體不足的問題。", - "suggestion": "建議限制 `delimiter` 嘗試次數，或在檢查時避免讀取整個 `output` 字串，改用串流處理方式。", - "is_new": true - }, - { - "level": "warning", - "role": "Mage", - "location": "app/main.js:175", - "problem": "在 `child.on('close', ...)` 事件中，使用 `Buffer.concat(outputChunks).toString()` 將所有輸出轉為單一字串。如果 `outputChunks` 總大小接近 `DEFAULT_OUTPUT_LIMIT_BYTES` (1MB)，這會導致瞬間記憶體使用量增加，且對於極大輸出，字串轉換本身亦有潛在的負載。", - "suggestion": "考慮使用 `Buffer` 處理後續輸出，或在達到 `outputLimitBytes` 時，僅保存 `Buffer` 片段即可，不必轉為大字串。", - "is_new": true - }, - { - "level": "warning", - "role": "Maya", - "location": "app/main.js:147", - "problem": "在 `runCodex` 函數中雖然有處理 `child.on('error', ...)`，但若 `codex` 指令本身不存在（spawn ENOENT），這裡捕捉到的 error stack trace 可能會包含完整的系統路徑資訊，這在 CI 環境中屬於資訊洩漏風險。", - "suggestion": "建議在錯誤處理中，針對 `error.code === 'ENOENT'` 做明確判斷，回傳簡潔的錯誤訊息（例如「找不到 codex 指令」），而非直接回傳完整的 `error.message`。", - "is_new": true - }, - { - "level": "warning", - "role": "Maya", - "location": "app/main.js:164", - "problem": "當 `child.on('close', ...)` 觸發時，若 `code` 為 null，預設回傳 1。雖然這處理了非預期終止，但缺少對 signal 終止（例如 SIGKILL）的具體紀錄，只知道失敗，無法區分是指令執行錯誤還是被系統殺掉。", - "suggestion": "在 `close` 事件中，若 `code` 為 null，可以檢查 `signal` 參數（若有），並在 output 中加入被哪個 signal 終止的資訊，增加除錯便利性。", - "is_new": true - }, - { - "level": "warning", - "role": "Rogue", - "location": "app/main.js:127", - "problem": "在處理輸出區塊時，使用 `chunks.reduce` 重複計算陣列大小，隨著資料量增加，這會造成不必要的 O(n²) 運算瓶頸，浪費 CPU 週期。", - "suggestion": "應在 closure 中維護一個 `currentSize` 變數來追蹤當前總大小，避免每次有新資料時都重新遍歷整個區塊陣列。", - "is_new": true - }, - { - "level": "warning", - "role": "Rogue", - "location": "app/main.js:166", - "problem": "在 `runCodex` 的輸出處理中，每收到一塊資料就進行 `Buffer.concat` 與 `toString`，若資料量大或封包碎，會產生大量不必要的記憶體配置與垃圾回收 (GC) 壓力。", - "suggestion": "只在輸出完成、達到限制或必須輸出結果時才進行合併與轉型，不要在處理每一塊資料時都執行。", - "is_new": true - }, - { - "level": "info", - "role": "Maya", - "location": "app/main.js:134", - "problem": "測試檔案 `tests/entrypoint_test.sh` 有測試 `missing_codex_command`，這很好。但實作中對於 `codex` 執行失敗的各種細節（如權限不足、找不到 binary 等）都統一處理為 `status: 1` 和簡單的訊息，測試僅驗證了 failure 狀態，未驗證具體錯誤來源。", - "suggestion": "考慮在 `main.js` 中根據不同的錯誤類型回傳更細緻的 status code，並在測試中驗證這些 code，能更精確地協助 CI 使用者除錯。", - "is_new": true - }, - { - "level": "info", - "role": "Rogue", - "location": "app/main.js:46", - "problem": "在 `TempFileRegistry` 的 `cleanup` 方法中，每次呼叫都使用 `Array.from` 將 Set 轉換為陣列，這在頻繁清理時會產生無謂的記憶體開銷。", - "suggestion": "若無強烈反向迭代的需求，可考慮直接使用 `forEach` 遍歷 Set。若有嚴格順序需求，建議改用其他結構管理，避免每次 cleanup 都額外配置陣列。", - "is_new": true - } -] +[]