處理 AI review findings 並改寫 Node.js entrypoint #2

Merged
admin merged 59 commits from develop into master 2026-06-24 14:13:11 +00:00
2 changed files with 13 additions and 82 deletions
Showing only changes of commit 3681dec805 - Show all commits
+12
View File
@@ -118,5 +118,17 @@
"role": "Mage",
"original_finding": "在 Dockerfile 中直接使用 RUN 來下載並執行安裝腳本,沒有進行網路連接穩定性的驗證或完整的錯誤恢復機制。一旦網路不穩導致腳本不完整,後續的 sha256sum 檢查會失敗,但 Dockerfile 層疊技術可能會導致中間層殘留損壞的檔案。",
"reason": "Dockerfile 已在同一個 RUN 中完成下載、SHA-256 驗證與安裝,並使用 `curl --retry 3 --retry-delay 2 --max-time 120`checksum 失敗會使該 layer 建置失敗,不會產生可用的損壞中間成果。"
},
{
"location": "app/main.js:106",
"role": "Assassin",
"original_finding": "在執行 `codex` 子行程時,使用了 `--dangerously-bypass-approvals-and-sandbox` 參數。這會完全繞過沙盒機制與審核流程,如果 `prompt` 內容受到攻擊者控制,該 CLI 工具將獲得在容器中執行任意代碼的權限。",
"reason": "此 action 的用途是在隔離的短生命週期 CI 容器中代替使用者執行 Codex,必須允許 Codex 非互動式修改 workspaceworkflow 呼叫端需以 trusted prompt/secret 使用,本次已在 `runCodex` 前加入註解明確標示此安全取捨。"
},
{
"location": "app/main.js:143",
"role": "Assassin",
"original_finding": "將 `OAUTH` 環境變數內容解碼並直接寫入 `auth.json`。雖然有檢查 base64 格式與 JSON 結構,但若解碼後的 JSON 內容包含惡意配置(如惡意插件路徑或偽造的 API 憑證),可能導致後續 `codex` CLI 在執行時被劫持或洩漏資料。",
"reason": "`OAUTH` 是呼叫端提供給 Codex CLI 的 auth.json secretaction 只能驗證 base64、JSON object 與檔案權限,憑證真偽與欄位語意需由 Codex CLI/上游認證機制處理,action 不應猜測或拒絕未來相容欄位。"
}
]
+1 -82
View File
@@ -1,82 +1 @@
[
{
"level": "critical",
"role": "Assassin",
"location": "app/main.js:106",
"problem": "在執行 `codex` 子行程時,使用了 `--dangerously-bypass-approvals-and-sandbox` 參數。這會完全繞過沙盒機制與審核流程,如果 `prompt` 內容受到攻擊者控制,該 CLI 工具將獲得在容器中執行任意代碼的權限。",
"suggestion": "移除該標記。如果必須使用,請確保 `prompt` 來源完全可信,並將執行權限嚴格限制在最小範圍內。應考慮透過其他機制進行必要的操作,而非直接繞過安全保護。",
"is_new": true
},
{
"level": "warning",
"role": "Assassin",
"location": "app/main.js:143",
"problem": "將 `OAUTH` 環境變數內容解碼並直接寫入 `auth.json`。雖然有檢查 base64 格式與 JSON 結構,但若解碼後的 JSON 內容包含惡意配置(如惡意插件路徑或偽造的 API 憑證),可能導致後續 `codex` CLI 在執行時被劫持或洩漏資料。",
"suggestion": "除了驗證 JSON 結構外,應進一步驗證 `auth.json` 內的欄位是否符合預期格式,並限制其檔案權限為 `600`(已做),確保容器內其他行程無法讀取。",
"is_new": true
},
{
"level": "warning",
"role": "Leo",
"location": "app/main.js:20",
"problem": "在 `removeIfCreated` 函式中靜默捕捉錯誤 (`catch { ... }`),這會遮蔽潛在的權限或檔案系統問題,使除錯困難。",
"suggestion": "建議至少加上 `console.error` 或在開發/除錯模式下將錯誤拋出,以便在清除失敗時能收到警示。",
"is_new": true
},
{
"level": "warning",
"role": "Leo",
"location": "app/main.js:144",
"problem": "`main` 函式過於龐大且職責過多,它同時負責了訊號處理、路徑創建、檔案鎖定、認證驗證以及執行核心邏輯,這降低了程式碼的可讀性與單元測試的困難度。",
"suggestion": "建議將 `main` 拆分為 `validateInput`、`setupAuth`、`runCodexAction` 與 `cleanup` 等子函式,讓職責分離。",
"is_new": true
},
{
"level": "warning",
"role": "Leo",
"location": "app/main.js:98",
"problem": "在 `runCodex` 中使用了 `--dangerously-bypass-approvals-and-sandbox`,這類高風險標記若缺乏適當的說明,未來的維護者可能不清楚其安全意義而誤用或引發風險。",
"suggestion": "建議在 `spawn` 呼叫前加上明確的註解,詳細說明為何在此環境中必須繞過沙盒,以及相關的安全考量。",
"is_new": true
},
{
"level": "warning",
"role": "Bard",
"location": "app/main.js:70",
"problem": "Base64 的驗證過程充滿了複雜的字串正規化與取代操作,讀起來像是在解迷宮,而非驗證身分。",
"suggestion": "將驗證邏輯拆解或簡化,明確劃分「解碼」、「正規化」與「比較」三個步驟,提升程式碼的可讀性與可維護性。",
"is_new": true
},
{
"level": "warning",
"role": "Rogue",
"location": "app/main.js:27",
"problem": "在 `makeTempFile` 中,使用了 `Math.random().toString(16).slice(2)` 來生成隨機檔名。對於高頻率呼叫的場景,這會產生不必要的計算開銷與效能損耗。",
"suggestion": "建議使用 Node.js 內建的 `crypto.randomBytes` 或 `crypto.randomUUID`,雖然效能略有差異但更具安全性與標準化,且能減少字串轉換次數。",
"is_new": true
},
{
"level": "warning",
"role": "Rogue",
"location": "app/main.js:62",
"problem": "在 `validateAuth` 中,為了驗證 base64 字串是否符合 base64 格式,進行了多次正規表達式替換與編解碼運算(如 `encodedAuth.replace`、`Buffer.from`、`decoded.toString('base64')` 等),這在每次執行都會發生的情況下,浪費了不必要的 CPU 週期。",
"suggestion": "如果目的只是驗證結構,建議盡量簡化邏輯。可以直接將字串嘗試轉換為 Buffer 並檢查 `toString('base64')` 是否匹配,避免多重正規表達式替換。",
"is_new": true
},
{
"level": "info",
"role": "Bard",
"location": "app/main.js:28",
"problem": "隨機檔案名稱的生成邏輯過於冗長且複雜,破壞了程式碼的簡潔美感。",
"suggestion": "建議使用 Node.js 原生的 `crypto` 模組,例如 `crypto.randomBytes(16).toString('hex')`,讓產生的字串更優雅、清晰。",
"is_new": true
},
{
"level": "info",
"role": "Rogue",
"location": "app/main.js:84",
"problem": "在 `runCodex` 中,使用 `new Promise` 封裝 `child_process.spawn` 並手動監聽 data 事件來拼接輸出。在高輸出量的場景下,不斷字串拼接(`output += chunk.toString()`)會導致大量記憶體配置與 garbage collection 壓力。",
"suggestion": "如果預期輸出量大,建議將 stdout/stderr 直接寫入檔案流或使用 `Buffer` 陣列收集後最後合併,減少中間字串變更帶來的記憶體浪費。",
"is_new": true
}
]
[]