處理 AI review findings 並改寫 Node.js entrypoint #2
@@ -130,5 +130,11 @@
|
||||
"role": "Assassin",
|
||||
"original_finding": "將 `OAUTH` 環境變數內容解碼並直接寫入 `auth.json`。雖然有檢查 base64 格式與 JSON 結構,但若解碼後的 JSON 內容包含惡意配置(如惡意插件路徑或偽造的 API 憑證),可能導致後續 `codex` CLI 在執行時被劫持或洩漏資料。",
|
||||
"reason": "`OAUTH` 是呼叫端提供給 Codex CLI 的 auth.json secret;action 只能驗證 base64、JSON object 與檔案權限,憑證真偽與欄位語意需由 Codex CLI/上游認證機制處理,action 不應猜測或拒絕未來相容欄位。"
|
||||
},
|
||||
{
|
||||
"location": "app/main.js:20",
|
||||
"role": "Leo",
|
||||
"original_finding": "建議至少加上 `console.error` 或在開發/除錯模式下將錯誤拋出,以便在清除失敗時能收到警示。",
|
||||
"reason": "AI 對話收斂判定為誤報(問題在最新程式碼中不成立或不適用)"
|
||||
}
|
||||
]
|
||||
|
||||
@@ -1 +1,106 @@
|
||||
[]
|
||||
[
|
||||
{
|
||||
"level": "critical",
|
||||
"role": "Mage",
|
||||
"location": "app/main.js:189",
|
||||
"problem": "在 `setupAuth` 中,`lockPath` 使用 `os.tmpdir()`。在共享環境中,如果 `CODEX_HOME` 字串相同,會導致所有 process 競爭同一個鎖檔,且如果其他無關的 process 也剛好在 `os.tmpdir()` 中建立相同名稱的檔案,會導致誤判或鎖定失敗。",
|
||||
"suggestion": "應在 `CODEX_HOME` 內部建立鎖檔,而非使用全域的 `os.tmpdir()`,或者包含更具唯一性的識別碼(如 PID 或更長的路徑雜湊)以確保鎖的隔離性。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Bard",
|
||||
"problem": "Base64 的驗證過程充滿了複雜的字串正規化與取代操作,讀起來像是在解迷宮,而非驗證身分。",
|
||||
"suggestion": "將驗證邏輯拆解或簡化,明確劃分「解碼」、「正規化」與「比較」三個步驟,提升程式碼的可讀性與可維護性。",
|
||||
"location": "app/main.js:70",
|
||||
"is_new": false
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Mage",
|
||||
"location": "app/main.js:33",
|
||||
"problem": "在 `makeTempFile` 中使用了 `fs.openSync(filePath, \"wx\", 0o600)`。如果在 `fs.closeSync(fd)` 之前程式因例外或強制終止(SIGKILL),該檔案會留在硬碟上直到下次清理或手動刪除,且其檔案描述子會持續開啟直到 process 結束。",
|
||||
"suggestion": "建議使用 `fs.mkdtempSync` 建立獨立目錄,將所有臨時檔案放入該目錄,並在 `cleanup` 時直接移除整個目錄,以確保清理的原子性與完整性。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Mage",
|
||||
"location": "app/main.js:77",
|
||||
"problem": "在 `validateAuth` 中,僅透過 `JSON.parse` 檢查 JSON 格式,但未針對 Codex 預期的 auth.json 結構(如必要的欄位)進行 Schema 驗證。如果傳入的 JSON 格式正確但內容無效,可能會導致 `codex exec` 在後續執行時失敗。",
|
||||
"suggestion": "建議加入對 JSON 內容的簡單結構驗證(例如確認是否有 `token` 或必要的連線設定欄位)。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Mage",
|
||||
"location": "app/main.js:114",
|
||||
"problem": "在 `runCodex` 中使用 `spawn` 時,沒有設定 `cwd`。如果 `codex` 工具依賴於當前工作目錄(例如需要編輯當前專案),這在 CI 環境中可能存在風險,雖然目前 CI 通常會設定好目錄,但這是一個隱含的契約。",
|
||||
"suggestion": "建議明確設定 `cwd` 為 `/github/workspace` 或 CI 定義的專案根目錄,確保 `codex` 運作在預期的上下文中。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Mage",
|
||||
"location": "app/main.js:203",
|
||||
"problem": "在 `setupAuth` 中,在 `fs.copyFileSync(authFile, authPath)` 後立即 `removeIfCreated(authFile)`,但在這期間如果發生 process 中斷,auth.json 可能會以不安全的權限(預設)或不完整的狀態寫入。",
|
||||
"suggestion": "建議使用 `fs.renameSync` 或在完成寫入與權限設定後再進行清理,並確保寫入過程中發生異常時能正確刪除該部分寫入的檔案。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Maya",
|
||||
"location": "app/main.js:102",
|
||||
"problem": "在執行外部指令時沒有設定逾時(timeout),若 Codex CLI 發生無預期的掛起(hang),Action 將會永久卡住而不會自動終止。",
|
||||
"suggestion": "建議在 `spawn` 的選項中加入 `timeout` 機制,或是主動在啟動後設置一個計時器,當執行時間過長時強制終止子行程。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Maya",
|
||||
"location": "app/main.js:125",
|
||||
"problem": "直接將所有輸出串接在 `outputChunks` 中,若 CLI 輸出過大的日誌,可能會導致記憶體耗盡(OOM)。",
|
||||
"suggestion": "建議針對 output 大小設定上限,超過限制時截斷輸出,或是改用串流寫入暫存檔以避免將所有內容存於記憶體。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Rogue",
|
||||
"location": "app/main.js:192",
|
||||
"problem": "在 setupAuth 中,`lockPath` 檔名產生使用了 `Buffer.from(codexHome).toString(\"hex\")`。如果 `codexHome` 非常長,這個檔名可能會超過作業系統的檔案名稱長度限制(通常為 255 bytes),導致鎖定失敗,進而阻斷整個流程。",
|
||||
"suggestion": "改用 `crypto.createHash('sha256').update(codexHome).digest('hex')` 來產生固定長度的雜湊值作為檔名的一部分,既安全又保證長度可控。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "info",
|
||||
"role": "Bard",
|
||||
"location": "app/main.js:11",
|
||||
"problem": "全域變數 `createdPaths` 在檔案層級被宣告,讓函式產生強依賴,缺乏封裝性,讀起來不夠優雅。",
|
||||
"suggestion": "將臨時檔案管理邏輯封裝成一個類別(如 `TempFileRegistry`),讓狀態更具備物件導向的封裝性。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "info",
|
||||
"role": "Bard",
|
||||
"location": "app/main.js:33",
|
||||
"problem": "檔案權限(如 `0o600`, `0o700`)以數字字面量多次出現,散落在程式碼中,降低了可讀性與一致性。",
|
||||
"suggestion": "在檔案上方定義權限常數(例如 `const FILE_MODE_PRIVATE = 0o600;`),讓語義更清晰。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "info",
|
||||
"role": "Maya",
|
||||
"location": "tests/docker_image_test.sh:20",
|
||||
"problem": "目前的 Docker 冒煙測試僅驗證了 CLI 二進位檔是否存在,但尚未驗證其在容器內執行時是否能正常存取與寫入 `CODEX_HOME` 環境設定的目錄。",
|
||||
"suggestion": "建議在 `docker_image_test.sh` 中增加一個測試案例,執行 `codex --version` 之外的指令,驗證容器權限與目錄環境變數設定是否正確。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "info",
|
||||
"role": "Rogue",
|
||||
"location": "app/main.js:109",
|
||||
"problem": "`runCodex` 函數中的 `spawn` 使用 `{ stdio: [\"ignore\", \"pipe\", \"pipe\"] }`,這會導致 node 程式在輸出流被填滿時阻塞等待,即便透過 `stdout.on('data')` 監聽,在高輸出的情境下仍可能因為緩衝區管理不當而浪費不必要的 CPU 週期。",
|
||||
"suggestion": "如果預期輸出量很大,建議改用 `child.stdout.pipe(process.stdout)` 直接導向,而非透過 node 的事件迴圈在兩者間搬運資料。",
|
||||
"is_new": true
|
||||
}
|
||||
]
|
||||
|
||||
Reference in New Issue
Block a user