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

Merged
jiantw83 merged 58 commits from ai-review-resolve/20260624102518 into develop 2026-06-24 14:09:27 +00:00
Showing only changes of commit 3f2355ed41 - Show all commits
+38 -12
View File
@@ -3,6 +3,7 @@
const fs = require("fs"); const fs = require("fs");
const os = require("os"); const os = require("os");
const path = require("path"); const path = require("path");
const crypto = require("crypto");
const { spawn } = require("child_process"); const { spawn } = require("child_process");
const DEFAULT_PROMPT = "請自我介紹"; const DEFAULT_PROMPT = "請自我介紹";
1
@@ -15,8 +16,8 @@ function removeIfCreated(filePath) {
try { try {
fs.rmSync(filePath, { force: true }); fs.rmSync(filePath, { force: true });
} catch { } catch (error) {
// Best-effort cleanup only. console.error(`Unable to remove temporary file: ${error.message}`);
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Leo
問題:在 removeIfCreated 函式中靜默捕捉錯誤 (catch { ... }),這會遮蔽潛在的權限或檔案系統問題,使除錯困難。
建議:建議至少加上 console.error 或在開發/除錯模式下將錯誤拋出,以便在清除失敗時能收到警示。

**嚴重等級**:🟡 警告 **審查員**:Leo **問題**:在 `removeIfCreated` 函式中靜默捕捉錯誤 (`catch { ... }`),這會遮蔽潛在的權限或檔案系統問題,使除錯困難。 **建議**:建議至少加上 `console.error` 或在開發/除錯模式下將錯誤拋出,以便在清除失敗時能收到警示。
} }
} }
1
@@ -27,7 +28,7 @@ function cleanup() {
} }
Ghost marked this conversation as resolved
Review

嚴重等級🔵 建議
審查員:Bard
問題:隨機檔案名稱的生成邏輯過於冗長且複雜,破壞了程式碼的簡潔美感。
建議:建議使用 Node.js 原生的 crypto 模組,例如 crypto.randomBytes(16).toString('hex'),讓產生的字串更優雅、清晰。

**嚴重等級**:🔵 建議 **審查員**:Bard **問題**:隨機檔案名稱的生成邏輯過於冗長且複雜,破壞了程式碼的簡潔美感。 **建議**:建議使用 Node.js 原生的 `crypto` 模組,例如 `crypto.randomBytes(16).toString('hex')`,讓產生的字串更優雅、清晰。
function makeTempFile(dir, prefix) { function makeTempFile(dir, prefix) {
const random = `${Date.now()}-${process.pid}-${Math.random().toString(16).slice(2)}`; const random = crypto.randomBytes(16).toString("hex");
const filePath = path.join(dir, `${prefix}.${random}`); const filePath = path.join(dir, `${prefix}.${random}`);
const fd = fs.openSync(filePath, "wx", 0o600); const fd = fs.openSync(filePath, "wx", 0o600);
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Mage
問題:在 makeTempFile 中使用了 fs.openSync(filePath, "wx", 0o600)。如果在 fs.closeSync(fd) 之前程式因例外或強制終止(SIGKILL),該檔案會留在硬碟上直到下次清理或手動刪除,且其檔案描述子會持續開啟直到 process 結束。
建議:建議使用 fs.mkdtempSync 建立獨立目錄,將所有臨時檔案放入該目錄,並在 cleanup 時直接移除整個目錄,以確保清理的原子性與完整性。

**嚴重等級**:🟡 警告 **審查員**:Mage **問題**:在 `makeTempFile` 中使用了 `fs.openSync(filePath, "wx", 0o600)`。如果在 `fs.closeSync(fd)` 之前程式因例外或強制終止(SIGKILL),該檔案會留在硬碟上直到下次清理或手動刪除,且其檔案描述子會持續開啟直到 process 結束。 **建議**:建議使用 `fs.mkdtempSync` 建立獨立目錄,將所有臨時檔案放入該目錄,並在 `cleanup` 時直接移除整個目錄,以確保清理的原子性與完整性。
Review

嚴重等級🔵 建議
審查員:Bard
問題:檔案權限(如 0o600, 0o700)以數字字面量多次出現,散落在程式碼中,降低了可讀性與一致性。
建議:在檔案上方定義權限常數(例如 const FILE_MODE_PRIVATE = 0o600;),讓語義更清晰。

**嚴重等級**:🔵 建議 **審查員**:Bard **問題**:檔案權限(如 `0o600`, `0o700`)以數字字面量多次出現,散落在程式碼中,降低了可讀性與一致性。 **建議**:在檔案上方定義權限常數(例如 `const FILE_MODE_PRIVATE = 0o600;`),讓語義更清晰。
fs.closeSync(fd); fs.closeSync(fd);
@@ -43,7 +44,7 @@ function appendGithubOutput(status, output) {
let delimiter; let delimiter;
do { do {
Ghost marked this conversation as resolved
Review

嚴重等級🔵 建議
審查員:Rogue
問題:在 TempFileRegistrycleanup 方法中,每次呼叫都使用 Array.from 將 Set 轉換為陣列,這在頻繁清理時會產生無謂的記憶體開銷。
建議:若無強烈反向迭代的需求,可考慮直接使用 forEach 遍歷 Set。若有嚴格順序需求,建議改用其他結構管理,避免每次 cleanup 都額外配置陣列。

**嚴重等級**:🔵 建議 **審查員**:Rogue **問題**:在 `TempFileRegistry` 的 `cleanup` 方法中,每次呼叫都使用 `Array.from` 將 Set 轉換為陣列,這在頻繁清理時會產生無謂的記憶體開銷。 **建議**:若無強烈反向迭代的需求,可考慮直接使用 `forEach` 遍歷 Set。若有嚴格順序需求,建議改用其他結構管理,避免每次 cleanup 都額外配置陣列。
delimiter = `CODEX_OUTPUT_${Math.random().toString(36).slice(2)}${Date.now()}`; delimiter = `CODEX_OUTPUT_${crypto.randomBytes(12).toString("hex")}`;
} while (output.includes(delimiter)); } while (output.includes(delimiter));
fs.appendFileSync( fs.appendFileSync(
@@ -61,14 +62,16 @@ function fail(message, code = 1) {
} }
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Rogue
問題:在 validateAuth 中,為了驗證 base64 字串是否符合 base64 格式,進行了多次正規表達式替換與編解碼運算(如 encodedAuth.replaceBuffer.fromdecoded.toString('base64') 等),這在每次執行都會發生的情況下,浪費了不必要的 CPU 週期。
建議:如果目的只是驗證結構,建議盡量簡化邏輯。可以直接將字串嘗試轉換為 Buffer 並檢查 toString('base64') 是否匹配,避免多重正規表達式替換。

**嚴重等級**:🟡 警告 **審查員**:Rogue **問題**:在 `validateAuth` 中,為了驗證 base64 字串是否符合 base64 格式,進行了多次正規表達式替換與編解碼運算(如 `encodedAuth.replace`、`Buffer.from`、`decoded.toString('base64')` 等),這在每次執行都會發生的情況下,浪費了不必要的 CPU 週期。 **建議**:如果目的只是驗證結構,建議盡量簡化邏輯。可以直接將字串嘗試轉換為 Buffer 並檢查 `toString('base64')` 是否匹配,避免多重正規表達式替換。
function validateAuth(encodedAuth, authFile) { function validateAuth(encodedAuth, authFile) {
const normalizedAuth = encodedAuth.replace(/\s+/g, "");
const decoded = Buffer.from(encodedAuth, "base64"); const decoded = Buffer.from(encodedAuth, "base64");
const normalizedDecoded = decoded.toString("base64").replace(/=+$/, "");
const normalizedInput = normalizedAuth.replace(/=+$/, "");
if (decoded.length === 0 && encodedAuth.length > 0) { if (decoded.length === 0 && normalizedAuth.length > 0) {
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Bard
問題:Base64 的驗證過程充滿了複雜的字串正規化與取代操作,讀起來像是在解迷宮,而非驗證身分。
建議:將驗證邏輯拆解或簡化,明確劃分「解碼」、「正規化」與「比較」三個步驟,提升程式碼的可讀性與可維護性。

**嚴重等級**:🟡 警告 **審查員**:Bard **問題**:Base64 的驗證過程充滿了複雜的字串正規化與取代操作,讀起來像是在解迷宮,而非驗證身分。 **建議**:將驗證邏輯拆解或簡化,明確劃分「解碼」、「正規化」與「比較」三個步驟,提升程式碼的可讀性與可維護性。
fail("OAUTH must be valid base64 encoded Codex auth.json."); fail("OAUTH must be valid base64 encoded Codex auth.json.");
} }
const normalized = encodedAuth.replace(/\s+/g, ""); if (normalizedDecoded !== normalizedInput) {
if (decoded.toString("base64").replace(/=+$/, "") !== normalized.replace(/=+$/, "")) {
fail("OAUTH must be valid base64 encoded Codex auth.json."); fail("OAUTH must be valid base64 encoded Codex auth.json.");
} }
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Mage
問題:在 validateAuth 中,僅透過 JSON.parse 檢查 JSON 格式,但未針對 Codex 預期的 auth.json 結構(如必要的欄位)進行 Schema 驗證。如果傳入的 JSON 格式正確但內容無效,可能會導致 codex exec 在後續執行時失敗。
建議:建議加入對 JSON 內容的簡單結構驗證(例如確認是否有 token 或必要的連線設定欄位)。

**嚴重等級**:🟡 警告 **審查員**:Mage **問題**:在 `validateAuth` 中,僅透過 `JSON.parse` 檢查 JSON 格式,但未針對 Codex 預期的 auth.json 結構(如必要的欄位)進行 Schema 驗證。如果傳入的 JSON 格式正確但內容無效,可能會導致 `codex exec` 在後續執行時失敗。 **建議**:建議加入對 JSON 內容的簡單結構驗證(例如確認是否有 `token` 或必要的連線設定欄位)。
Review

嚴重等級🟡 警告
審查員:Mage
問題:在 appendGithubOutput 函式中,當 output 內容極大時,此處會將整個 output 字串在記憶體中進行檢查(output.includes(delimiter))與多次複製。這可能導致在處理極端長度輸出時發生記憶體不足的問題。
建議:建議限制 delimiter 嘗試次數,或在檢查時避免讀取整個 output 字串,改用串流處理方式。

**嚴重等級**:🟡 警告 **審查員**:Mage **問題**:在 `appendGithubOutput` 函式中,當 `output` 內容極大時,此處會將整個 `output` 字串在記憶體中進行檢查(`output.includes(delimiter)`)與多次複製。這可能導致在處理極端長度輸出時發生記憶體不足的問題。 **建議**:建議限制 `delimiter` 嘗試次數,或在檢查時避免讀取整個 `output` 字串,改用串流處理方式。
Review

嚴重等級🟡 警告
審查員:Maya
問題:OutputCollector 的截斷機制未被測試。
建議:增加測試案例模擬輸出超過 DEFAULT_OUTPUT_LIMIT_BYTES,驗證截斷提示。

**嚴重等級**:🟡 警告 **審查員**:Maya **問題**:OutputCollector 的截斷機制未被測試。 **建議**:增加測試案例模擬輸出超過 DEFAULT_OUTPUT_LIMIT_BYTES,驗證截斷提示。
1
@@ -88,6 +91,8 @@ function validateAuth(encodedAuth, authFile) {
function runCodex(model, prompt) { function runCodex(model, prompt) {
return new Promise((resolve) => { return new Promise((resolve) => {
Ghost marked this conversation as resolved
Review

嚴重等級🔵 建議
審查員:Bard
問題:makeTempDir 內部使用了硬編碼的 '.codex-action-' 前綴。
建議:將前綴提取為常數或設定檔參數。

**嚴重等級**:🔵 建議 **審查員**:Bard **問題**:makeTempDir 內部使用了硬編碼的 '.codex-action-' 前綴。 **建議**:將前綴提取為常數或設定檔參數。
// This Docker Action runs inside an ephemeral CI container where Codex must be
// able to edit the checked-out workspace without interactive approvals.
const child = spawn( const child = spawn(
"codex", "codex",
[ [
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Leo
問題:在 runCodex 中使用了 --dangerously-bypass-approvals-and-sandbox,這類高風險標記若缺乏適當的說明,未來的維護者可能不清楚其安全意義而誤用或引發風險。
建議:建議在 spawn 呼叫前加上明確的註解,詳細說明為何在此環境中必須繞過沙盒,以及相關的安全考量。

**嚴重等級**:🟡 警告 **審查員**:Leo **問題**:在 `runCodex` 中使用了 `--dangerously-bypass-approvals-and-sandbox`,這類高風險標記若缺乏適當的說明,未來的維護者可能不清楚其安全意義而誤用或引發風險。 **建議**:建議在 `spawn` 呼叫前加上明確的註解,詳細說明為何在此環境中必須繞過沙盒,以及相關的安全考量。
1
@@ -101,30 +106,35 @@ function runCodex(model, prompt) {
{ stdio: ["ignore", "pipe", "pipe"] }, { stdio: ["ignore", "pipe", "pipe"] },
Ghost marked this conversation as resolved
Review

嚴重等級🔴 嚴重
審查員:Assassin
問題:在執行 codex 子行程時,使用了 --dangerously-bypass-approvals-and-sandbox 參數。這會完全繞過沙盒機制與審核流程,如果 prompt 內容受到攻擊者控制,該 CLI 工具將獲得在容器中執行任意代碼的權限。
建議:移除該標記。如果必須使用,請確保 prompt 來源完全可信,並將執行權限嚴格限制在最小範圍內。應考慮透過其他機制進行必要的操作,而非直接繞過安全保護。

**嚴重等級**:🔴 嚴重 **審查員**:Assassin **問題**:在執行 `codex` 子行程時,使用了 `--dangerously-bypass-approvals-and-sandbox` 參數。這會完全繞過沙盒機制與審核流程,如果 `prompt` 內容受到攻擊者控制,該 CLI 工具將獲得在容器中執行任意代碼的權限。 **建議**:移除該標記。如果必須使用,請確保 `prompt` 來源完全可信,並將執行權限嚴格限制在最小範圍內。應考慮透過其他機制進行必要的操作,而非直接繞過安全保護。
); );
let output = ""; const outputChunks = [];
Ghost marked this conversation as resolved
Review

嚴重等級🔵 建議
審查員:Rogue
問題runCodex 函數中的 spawn 使用 { stdio: ["ignore", "pipe", "pipe"] },這會導致 node 程式在輸出流被填滿時阻塞等待,即便透過 stdout.on('data') 監聽,在高輸出的情境下仍可能因為緩衝區管理不當而浪費不必要的 CPU 週期。
建議:如果預期輸出量很大,建議改用 child.stdout.pipe(process.stdout) 直接導向,而非透過 node 的事件迴圈在兩者間搬運資料。

**嚴重等級**:🔵 建議 **審查員**:Rogue **問題**:`runCodex` 函數中的 `spawn` 使用 `{ stdio: ["ignore", "pipe", "pipe"] }`,這會導致 node 程式在輸出流被填滿時阻塞等待,即便透過 `stdout.on('data')` 監聽,在高輸出的情境下仍可能因為緩衝區管理不當而浪費不必要的 CPU 週期。 **建議**:如果預期輸出量很大,建議改用 `child.stdout.pipe(process.stdout)` 直接導向,而非透過 node 的事件迴圈在兩者間搬運資料。
const appendOutput = (chunk) => {
outputChunks.push(chunk);
return Buffer.concat(outputChunks).toString();
};
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Mage
問題:在 runCodex 中使用 spawn 時,沒有設定 cwd。如果 codex 工具依賴於當前工作目錄(例如需要編輯當前專案),這在 CI 環境中可能存在風險,雖然目前 CI 通常會設定好目錄,但這是一個隱含的契約。
建議:建議明確設定 cwd/github/workspace 或 CI 定義的專案根目錄,確保 codex 運作在預期的上下文中。

**嚴重等級**:🟡 警告 **審查員**:Mage **問題**:在 `runCodex` 中使用 `spawn` 時,沒有設定 `cwd`。如果 `codex` 工具依賴於當前工作目錄(例如需要編輯當前專案),這在 CI 環境中可能存在風險,雖然目前 CI 通常會設定好目錄,但這是一個隱含的契約。 **建議**:建議明確設定 `cwd` 為 `/github/workspace` 或 CI 定義的專案根目錄,確保 `codex` 運作在預期的上下文中。
child.stdout.on("data", (chunk) => { child.stdout.on("data", (chunk) => {
process.stdout.write(chunk); process.stdout.write(chunk);
output += chunk.toString(); outputChunks.push(chunk);
}); });
child.stderr.on("data", (chunk) => { child.stderr.on("data", (chunk) => {
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Bard
問題:validateAuth 函式中對於 Base64 的正規化與驗證邏輯混雜在一起,使用了大量的取代與判斷,讀起來節奏凌亂,缺乏優雅感。
建議:將驗證邏輯與基礎轉換邏輯抽離,建議提取一個輔助函式專門負責 Base64 格式檢查,使主要流程清晰明瞭。

**嚴重等級**:🟡 警告 **審查員**:Bard **問題**:validateAuth 函式中對於 Base64 的正規化與驗證邏輯混雜在一起,使用了大量的取代與判斷,讀起來節奏凌亂,缺乏優雅感。 **建議**:將驗證邏輯與基礎轉換邏輯抽離,建議提取一個輔助函式專門負責 Base64 格式檢查,使主要流程清晰明瞭。
process.stdout.write(chunk); process.stdout.write(chunk);
output += chunk.toString(); outputChunks.push(chunk);
}); });
child.on("error", (error) => { child.on("error", (error) => {
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Maya
問題:直接將所有輸出串接在 outputChunks 中,若 CLI 輸出過大的日誌,可能會導致記憶體耗盡(OOM)。
建議:建議針對 output 大小設定上限,超過限制時截斷輸出,或是改用串流寫入暫存檔以避免將所有內容存於記憶體。

**嚴重等級**:🟡 警告 **審查員**:Maya **問題**:直接將所有輸出串接在 `outputChunks` 中,若 CLI 輸出過大的日誌,可能會導致記憶體耗盡(OOM)。 **建議**:建議針對 output 大小設定上限,超過限制時截斷輸出,或是改用串流寫入暫存檔以避免將所有內容存於記憶體。
Review

嚴重等級🔴 嚴重
審查員:Mage
問題:這裡直接使用 spawn 執行 codex 命令,且參數 prompt 是直接從 process.env.PROMPT 讀取並傳入的。如果 CI 環境的 PROMPT 被惡意竄改,雖使用陣列傳遞參數避免了 shell injection,但 codex exec 的邏輯若沒有妥善限制(例如限制可執行指令類型),可能導致攻擊者在 CI Runner 環境執行任意指令。
建議:在 runCodex 函式中,除了已經加入的 --dangerously-bypass-approvals-and-sandbox 外,必須確保對 prompt 進行強力的白名單過濾,或改為使用非 exec 的子指令來限制權限。

**嚴重等級**:🔴 嚴重 **審查員**:Mage **問題**:這裡直接使用 `spawn` 執行 `codex` 命令,且參數 `prompt` 是直接從 `process.env.PROMPT` 讀取並傳入的。如果 CI 環境的 `PROMPT` 被惡意竄改,雖使用陣列傳遞參數避免了 shell injection,但 `codex exec` 的邏輯若沒有妥善限制(例如限制可執行指令類型),可能導致攻擊者在 CI Runner 環境執行任意指令。 **建議**:在 `runCodex` 函式中,除了已經加入的 `--dangerously-bypass-approvals-and-sandbox` 外,必須確保對 `prompt` 進行強力的白名單過濾,或改為使用非 `exec` 的子指令來限制權限。
output += `${error.message}\n`; const output = appendOutput(Buffer.from(`${error.message}\n`));
resolve({ status: 1, output }); resolve({ status: 1, output });
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Rogue
問題:在處理輸出區塊時,使用 chunks.reduce 重複計算陣列大小,隨著資料量增加,這會造成不必要的 O(n²) 運算瓶頸,浪費 CPU 週期。
建議:應在 closure 中維護一個 currentSize 變數來追蹤當前總大小,避免每次有新資料時都重新遍歷整個區塊陣列。

**嚴重等級**:🟡 警告 **審查員**:Rogue **問題**:在處理輸出區塊時,使用 `chunks.reduce` 重複計算陣列大小,隨著資料量增加,這會造成不必要的 O(n²) 運算瓶頸,浪費 CPU 週期。 **建議**:應在 closure 中維護一個 `currentSize` 變數來追蹤當前總大小,避免每次有新資料時都重新遍歷整個區塊陣列。
Review

嚴重等級🟡 警告
審查員:Rogue
問題:處理輸出區塊時使用 chunks.reduce 重複計算陣列大小,造成 O(n²) 運算瓶頸。
建議:在 closure 中維護 currentSize 變數追蹤總大小,避免遍歷。

**嚴重等級**:🟡 警告 **審查員**:Rogue **問題**:處理輸出區塊時使用 chunks.reduce 重複計算陣列大小,造成 O(n²) 運算瓶頸。 **建議**:在 closure 中維護 currentSize 變數追蹤總大小,避免遍歷。
}); });
child.on("close", (code) => { child.on("close", (code) => {
const output = Buffer.concat(outputChunks).toString();
resolve({ status: code ?? 1, output }); resolve({ status: code ?? 1, output });
}); });
}); });
Ghost marked this conversation as resolved
Review

嚴重等級🔵 建議
審查員:Maya
問題:測試檔案 tests/entrypoint_test.sh 有測試 missing_codex_command,這很好。但實作中對於 codex 執行失敗的各種細節(如權限不足、找不到 binary 等)都統一處理為 status: 1 和簡單的訊息,測試僅驗證了 failure 狀態,未驗證具體錯誤來源。
建議:考慮在 main.js 中根據不同的錯誤類型回傳更細緻的 status code,並在測試中驗證這些 code,能更精確地協助 CI 使用者除錯。

**嚴重等級**:🔵 建議 **審查員**:Maya **問題**:測試檔案 `tests/entrypoint_test.sh` 有測試 `missing_codex_command`,這很好。但實作中對於 `codex` 執行失敗的各種細節(如權限不足、找不到 binary 等)都統一處理為 `status: 1` 和簡單的訊息,測試僅驗證了 failure 狀態,未驗證具體錯誤來源。 **建議**:考慮在 `main.js` 中根據不同的錯誤類型回傳更細緻的 status code,並在測試中驗證這些 code,能更精確地協助 CI 使用者除錯。
Review

嚴重等級🔵 建議
審查員:Maya
問題:對於 codex 執行失敗的各種細節(權限不足、找不到 binary 等)都統一處理為 status: 1,測試未驗證具體錯誤來源。
建議:根據錯誤類型回傳細緻 status code,並在測試中驗證這些 code。

**嚴重等級**:🔵 建議 **審查員**:Maya **問題**:對於 codex 執行失敗的各種細節(權限不足、找不到 binary 等)都統一處理為 status: 1,測試未驗證具體錯誤來源。 **建議**:根據錯誤類型回傳細緻 status code,並在測試中驗證這些 code。
} }
async function main() { function registerCleanupHandlers() {
process.on("exit", cleanup); process.on("exit", cleanup);
process.on("SIGINT", () => { process.on("SIGINT", () => {
cleanup(); cleanup();
2
@@ -134,7 +144,9 @@ async function main() {
cleanup(); cleanup();
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Leo
問題main 函式過於龐大且職責過多,它同時負責了訊號處理、路徑創建、檔案鎖定、認證驗證以及執行核心邏輯,這降低了程式碼的可讀性與單元測試的困難度。
建議:建議將 main 拆分為 validateInputsetupAuthrunCodexActioncleanup 等子函式,讓職責分離。

**嚴重等級**:🟡 警告 **審查員**:Leo **問題**:`main` 函式過於龐大且職責過多,它同時負責了訊號處理、路徑創建、檔案鎖定、認證驗證以及執行核心邏輯,這降低了程式碼的可讀性與單元測試的困難度。 **建議**:建議將 `main` 拆分為 `validateInput`、`setupAuth`、`runCodexAction` 與 `cleanup` 等子函式,讓職責分離。
process.exit(143); process.exit(143);
}); });
}
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Maya
問題:在 runCodex 函數中雖然有處理 child.on('error', ...),但若 codex 指令本身不存在(spawn ENOENT),這裡捕捉到的 error stack trace 可能會包含完整的系統路徑資訊,這在 CI 環境中屬於資訊洩漏風險。
建議:建議在錯誤處理中,針對 error.code === 'ENOENT' 做明確判斷,回傳簡潔的錯誤訊息(例如「找不到 codex 指令」),而非直接回傳完整的 error.message

**嚴重等級**:🟡 警告 **審查員**:Maya **問題**:在 `runCodex` 函數中雖然有處理 `child.on('error', ...)`,但若 `codex` 指令本身不存在(spawn ENOENT),這裡捕捉到的 error stack trace 可能會包含完整的系統路徑資訊,這在 CI 環境中屬於資訊洩漏風險。 **建議**:建議在錯誤處理中,針對 `error.code === 'ENOENT'` 做明確判斷,回傳簡潔的錯誤訊息(例如「找不到 codex 指令」),而非直接回傳完整的 `error.message`。
Review

嚴重等級🟡 警告
審查員:Maya
問題:spawn ENOENT 錯誤處理可能洩漏系統路徑資訊。
建議:針對 error.code === 'ENOENT' 做明確判斷,回傳簡潔錯誤訊息而非完整 stack trace。

**嚴重等級**:🟡 警告 **審查員**:Maya **問題**:spawn ENOENT 錯誤處理可能洩漏系統路徑資訊。 **建議**:針對 error.code === 'ENOENT' 做明確判斷,回傳簡潔錯誤訊息而非完整 stack trace。
function readConfig() {
const oauth = process.env.OAUTH || ""; const oauth = process.env.OAUTH || "";
const model = process.env.MODEL || ""; const model = process.env.MODEL || "";
const codexHome = process.env.CODEX_HOME || "/root/.codex"; const codexHome = process.env.CODEX_HOME || "/root/.codex";
@@ -148,6 +160,10 @@ async function main() {
fail("MODEL is required."); fail("MODEL is required.");
} }
return { oauth, model, codexHome, prompt };
}
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Maya
問題:當 child.on('close', ...) 觸發時,若 code 為 null,預設回傳 1。雖然這處理了非預期終止,但缺少對 signal 終止(例如 SIGKILL)的具體紀錄,只知道失敗,無法區分是指令執行錯誤還是被系統殺掉。
建議:在 close 事件中,若 code 為 null,可以檢查 signal 參數(若有),並在 output 中加入被哪個 signal 終止的資訊,增加除錯便利性。

**嚴重等級**:🟡 警告 **審查員**:Maya **問題**:當 `child.on('close', ...)` 觸發時,若 `code` 為 null,預設回傳 1。雖然這處理了非預期終止,但缺少對 signal 終止(例如 SIGKILL)的具體紀錄,只知道失敗,無法區分是指令執行錯誤還是被系統殺掉。 **建議**:在 `close` 事件中,若 `code` 為 null,可以檢查 `signal` 參數(若有),並在 output 中加入被哪個 signal 終止的資訊,增加除錯便利性。
Review

嚴重等級🟡 警告
審查員:Maya
問題:close 事件缺少對 signal 終止(如 SIGKILL)的具體紀錄。
建議:檢查 signal 參數,並在 output 中加入被哪個 signal 終止的資訊。

**嚴重等級**:🟡 警告 **審查員**:Maya **問題**:close 事件缺少對 signal 終止(如 SIGKILL)的具體紀錄。 **建議**:檢查 signal 參數,並在 output 中加入被哪個 signal 終止的資訊。
function setupAuth(oauth, codexHome) {
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Bard
問題:runCodex 函式過於臃腫,包含了執行、超時處理、輸出截斷與錯誤捕捉等多重責任,這段旋律太過冗長且複雜。
建議:建議將輸出處理 (Output truncation logic) 與超時設定分離為獨立函式,以提升函式的可讀性與維護性。

**嚴重等級**:🟡 警告 **審查員**:Bard **問題**:runCodex 函式過於臃腫,包含了執行、超時處理、輸出截斷與錯誤捕捉等多重責任,這段旋律太過冗長且複雜。 **建議**:建議將輸出處理 (Output truncation logic) 與超時設定分離為獨立函式,以提升函式的可讀性與維護性。
Review

嚴重等級🟡 警告
審查員:Rogue
問題:在 runCodex 的輸出處理中,每收到一塊資料就進行 Buffer.concattoString,若資料量大或封包碎,會產生大量不必要的記憶體配置與垃圾回收 (GC) 壓力。
建議:只在輸出完成、達到限制或必須輸出結果時才進行合併與轉型,不要在處理每一塊資料時都執行。

**嚴重等級**:🟡 警告 **審查員**:Rogue **問題**:在 `runCodex` 的輸出處理中,每收到一塊資料就進行 `Buffer.concat` 與 `toString`,若資料量大或封包碎,會產生大量不必要的記憶體配置與垃圾回收 (GC) 壓力。 **建議**:只在輸出完成、達到限制或必須輸出結果時才進行合併與轉型,不要在處理每一塊資料時都執行。
Review

嚴重等級🟡 警告
審查員:Bard
問題:runCodex 函式過於臃腫,包含了過多職責。
建議:建議將輸出處理與超時設定分離為獨立函式。

**嚴重等級**:🟡 警告 **審查員**:Bard **問題**:runCodex 函式過於臃腫,包含了過多職責。 **建議**:建議將輸出處理與超時設定分離為獨立函式。
try { try {
fs.mkdirSync(codexHome, { recursive: true, mode: 0o700 }); fs.mkdirSync(codexHome, { recursive: true, mode: 0o700 });
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Maya
問題:parsePositiveInteger 的輸入回退機制未經測試。
建議:針對設定變數傳入無效數字或非法格式場景,驗證預設值套用。

**嚴重等級**:🟡 警告 **審查員**:Maya **問題**:parsePositiveInteger 的輸入回退機制未經測試。 **建議**:針對設定變數傳入無效數字或非法格式場景,驗證預設值套用。
} catch { } catch {
5
@@ -177,6 +193,11 @@ async function main() {
createdPaths.add(authPath); createdPaths.add(authPath);
removeIfCreated(authFile); removeIfCreated(authFile);
return lockHandle;
}
async function runCodexAction({ oauth, model, codexHome, prompt }) {
const lockHandle = setupAuth(oauth, codexHome);
const result = await runCodex(model, prompt); const result = await runCodex(model, prompt);
appendGithubOutput(result.status === 0 ? "completed" : "failed", result.output); appendGithubOutput(result.status === 0 ? "completed" : "failed", result.output);
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Mage
問題:在 setupAuth 中,在 fs.copyFileSync(authFile, authPath) 後立即 removeIfCreated(authFile),但在這期間如果發生 process 中斷,auth.json 可能會以不安全的權限(預設)或不完整的狀態寫入。
建議:建議使用 fs.renameSync 或在完成寫入與權限設定後再進行清理,並確保寫入過程中發生異常時能正確刪除該部分寫入的檔案。

**嚴重等級**:🟡 警告 **審查員**:Mage **問題**:在 `setupAuth` 中,在 `fs.copyFileSync(authFile, authPath)` 後立即 `removeIfCreated(authFile)`,但在這期間如果發生 process 中斷,auth.json 可能會以不安全的權限(預設)或不完整的狀態寫入。 **建議**:建議使用 `fs.renameSync` 或在完成寫入與權限設定後再進行清理,並確保寫入過程中發生異常時能正確刪除該部分寫入的檔案。
@@ -188,6 +209,11 @@ async function main() {
process.exit(result.status); process.exit(result.status);
} }
async function main() {
registerCleanupHandlers();
await runCodexAction(readConfig());
}
main().catch((error) => { main().catch((error) => {
fail(error instanceof Error ? error.message : String(error)); fail(error instanceof Error ? error.message : String(error));
}); });