處理 AI review findings 並改寫 Node.js entrypoint #1
@@ -9,6 +9,7 @@ die() {
|
||||
|
||||
cleanup() {
|
||||
rm -f "${auth_file:-}" "${auth_path:-}" "${codex_output:-}"
|
||||
|
Ghost marked this conversation as resolved
Outdated
|
||||
rmdir "${auth_lock:-}" 2>/dev/null || true
|
||||
}
|
||||
|
||||
trap cleanup EXIT
|
||||
@@ -24,13 +25,13 @@ fi
|
||||
CODEX_HOME="${CODEX_HOME:-/root/.codex}"
|
||||
PROMPT="${PROMPT:-請自我介紹}"
|
||||
|
Ghost marked this conversation as resolved
Outdated
gitea-actions
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Mage
**問題**:使用了 `mktemp` 建立 `auth_file`,但隨後直接透過 `mv` 將其移動到 `CODEX_HOME/auth.json`。如果目標檔案已存在,此操作會覆寫且若權限設定不當會造成安全隱患;且 `trap` 在 `mv` 後已移除,若處理中斷可能留下暫存檔。
**建議**:建議直接將 base64 解碼內容寫入 `$CODEX_HOME/auth.json`,並在寫入前先設定好目錄權限,或使用 `install -m 600` 指令來確保原子性與權限安全。
|
||||
mkdir -p "$CODEX_HOME" || die "Unable to create CODEX_HOME."
|
||||
|
Ghost marked this conversation as resolved
Outdated
gitea-actions
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Mage
**問題**:若 `$OAUTH` 的值開頭為 `-`,`printf '%s' "$OAUTH"` 會被 `printf` 解析為參數選項,導致無法正確輸出內容。
**建議**:改用 `printf '%s
' "$OAUTH"` 或其他不會將變數內容解析為選項的方式。
gitea-actions
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Maya
**問題**:解碼後的 auth.json 格式驗證(jq 檢查)未被測試,若輸入無效 JSON 或非 object,目前行為是否如預期未驗證。
**建議**:應補上測試案例,傳入無效的 Base64 字串或解碼後非 JSON 的內容,驗證腳本能否正確捕獲錯誤。
|
||||
umask 077
|
||||
|
||||
auth_file="$(mktemp "$CODEX_HOME/auth.XXXXXX")"
|
||||
auth_path="$CODEX_HOME/auth.json"
|
||||
|
Ghost marked this conversation as resolved
Outdated
gitea-actions
commented
嚴重等級:🔴 嚴重 **嚴重等級**:🔴 嚴重
**審查員**:Leo
**問題**:在腳本中使用了 `mv` 指令將臨時產生的 `auth.json` 移至 `$CODEX_HOME/auth.json`。若在此之前 `$CODEX_HOME/auth.json` 已經存在,`mv` 會強制覆蓋,這可能會導致隱蔽的配置遺失,且這種副作用在腳本執行中非常危險,不利於除錯。
**建議**:在寫入設定檔前,應先檢查目標檔案是否存在,並根據業務需求決定是要備份、合併或拋出錯誤,避免無意間覆蓋掉重要的設定。
|
||||
auth_lock="$CODEX_HOME/auth.lock"
|
||||
|
||||
if [[ -e "$auth_path" ]]; then
|
||||
die "Refusing to overwrite existing Codex auth.json."
|
||||
fi
|
||||
mkdir "$auth_lock" || die "Unable to lock Codex auth.json."
|
||||
|
||||
|
Ghost marked this conversation as resolved
gitea-actions
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Mage
**問題**:使用 `mkdir` 作為鎖定機制若容器意外崩潰可能殘留鎖檔。
**建議**:考慮使用更穩健的 `flock` 機制來管理檔案鎖並確保自動清理。
|
||||
if ! printf '%s\n' "$OAUTH" | base64 -d > "$auth_file"; then
|
||||
|
Ghost marked this conversation as resolved
Outdated
gitea-actions
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Leo
**問題**:直接執行 `codex exec` 而未對其可能的執行失敗進行顯式的錯誤捕捉。若該指令失敗,腳本仍會繼續執行後續步驟(例如設定 GitHub Output),這會導致對外回報錯誤的狀態碼不一致。
**建議**:應對 `codex exec` 進行錯誤檢查(使用 `if ! codex exec ...; then ... fi`),確保在失敗時能正確終止腳本並輸出相關錯誤訊息。
gitea-actions
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Mage
**問題**:`mv` 指令未檢查目標檔案是否已存在,無條件覆蓋會導致舊有的有效設定直接遺失。
**建議**:在 `mv` 前加入檢查,若 `auth.json` 已存在,應決定是否備份、報錯,或確認無須保留舊檔。
|
||||
die "OAUTH must be valid base64 encoded Codex auth.json."
|
||||
|
Ghost marked this conversation as resolved
Outdated
gitea-actions
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Mage
**問題**:執行 `codex exec` 的邏輯中使用了 `set +e` 暫時關閉錯誤退出機制,儘管後續透過 `PIPESTATUS` 檢查,但若 `codex exec` 指令本身因為環境錯誤或語法錯誤無法啟動,`codex_status` 可能會取得非預期的狀態碼。
**建議**:明確定義各個步驟的錯誤處理,或者將 `codex exec` 包裝在明確的函數中檢查退出碼。
|
||||
@@ -40,26 +41,41 @@ if ! jq -e 'type == "object"' "$auth_file" >/dev/null; then
|
||||
die "Decoded OAUTH must be a JSON object."
|
||||
fi
|
||||
|
Ghost marked this conversation as resolved
gitea-actions
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Mage
**問題**:使用 `install -m 600` 將 `auth_file` 移至 `auth_path`。若 `mktemp` 產生的 `auth_file` 與 `auth_path` 不在同一個檔案系統分區(Filesystem),`install` 指令(底層通常是 copy + chmod/chown)可能會有短暫時間檔案權限為預設值,存在權限外洩風險。
**建議**:建議在確認檔案權限無誤後,於同一分區內使用 `mv` 進行原子性移轉,或在寫入前先明確設定 `umask`。
|
||||
|
||||
if [[ -e "$auth_path" ]]; then
|
||||
die "Refusing to overwrite existing Codex auth.json."
|
||||
|
Ghost marked this conversation as resolved
Outdated
gitea-actions
commented
嚴重等級:🔵 建議 **嚴重等級**:🔵 建議
**審查員**:Mage
**問題**:在 GitHub Actions 輸出處理中,使用了 `date +%s` 來產生 output delimiter。雖然發生機率極低,但在極高併發或相同執行時間下可能發生碰撞導致輸出被截斷。
**建議**:建議使用更具隨機性的 UUID 或確保 delimiter 包含隨機字串,避免與內容衝突。
gitea-actions
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Bard
**問題**:使用靜態路徑檢查 `auth.json` 是否存在,若前次執行中斷導致檔案未清除,會導致後續執行失敗(Self-inflicted DoS)。
**建議**:移除對既有檔案的檢查,改為在執行前確保該檔案為最新且受控狀態,或使用唯一的隨機臨時檔。
|
||||
fi
|
||||
|
||||
install -m 600 "$auth_file" "$auth_path"
|
||||
rm -f "$auth_file"
|
||||
|
Ghost marked this conversation as resolved
gitea-actions
commented
嚴重等級:🔴 嚴重 **嚴重等級**:🔴 嚴重
**審查員**:Mage
**問題**:在執行 install -m 600 時,若 codex 進程已經在嘗試讀取 auth.json,會發生檔案存取競態(Race Condition)。雖然使用了 flock,但這僅在同一個 shell 腳本實例中有效,無法保護跨容器或跨執行環境的檔案存取一致性。
**建議**:建議將 auth.json 放置於唯讀且受限的目錄中,並通過環境變數直接傳遞路徑給 codex,而非在執行時進行檔案寫入與複製。
|
||||
|
||||
codex_output="$(mktemp)"
|
||||
|
||||
run_codex() {
|
||||
set +e
|
||||
codex exec \
|
||||
--dangerously-bypass-approvals-and-sandbox \
|
||||
--skip-git-repo-check \
|
||||
--model "$MODEL" \
|
||||
|
Ghost marked this conversation as resolved
Outdated
gitea-actions
commented
嚴重等級:🔴 嚴重 **嚴重等級**:🔴 嚴重
**審查員**:Mage
**問題**:在併發環境下,若多個執行緒或過程同時嘗試建立 auth.json,可能會因為檢查檔案是否存在(Line 35)與建立檔案之間的競態條件,導致 `die` 錯誤甚至意外地驗證失敗。雖然目前看起來是單一容器環境,但在 GitHub Actions 或其他 Runner 中,安全起見應使用原子操作。
**建議**:建議使用 `mkdir` 的原子性或檔案鎖定機制,或是確保 `auth_path` 在容器初始化階段就已經是唯讀且受保護的,避免檢查與寫入之間的延遲風險。
gitea-actions
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Mage
**問題**:在容器化環境(通常是 ephemeral 的)中,auth.json 寫入後立刻被刪除,這會導致 codex 在後續執行中因找不到驗證檔案而無法運作。另外,trap 的清理機制會導致該檔案在 codex 完成工作前被刪除,這對於長效執行或需要多次存取的應用場景是錯誤的設計。
**建議**:評估 codex 是否需要該檔案在執行期間持續存在。若需要,請調整清理時機,或考慮使用記憶體中的臨時檔案系統(tmpfs)來提升安全性,而非直接移除檔案。
|
||||
"$PROMPT" 2>&1 | tee "$codex_output"
|
||||
codex_status="${PIPESTATUS[0]}"
|
||||
local status="${PIPESTATUS[0]}"
|
||||
set -e
|
||||
return "$status"
|
||||
}
|
||||
|
||||
if run_codex; then
|
||||
|
Ghost marked this conversation as resolved
Outdated
gitea-actions
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Bard
**問題**:在執行 `codex` 的函式中,使用 `set +e` 與 `set -e` 的開關切換來處理回傳值,雖然正確但破壞了程式碼的流暢閱讀感,像是在樂譜中頻繁變調。
**建議**:建議嘗試使用 `if ! codex ...; then status=$?; fi` 的方式,避免在函式內頻繁切換 `set -e` 狀態,保持程式邏輯的單純性。
|
||||
codex_status=0
|
||||
else
|
||||
codex_status="$?"
|
||||
fi
|
||||
|
||||
if [[ -n "${GITHUB_OUTPUT:-}" ]]; then
|
||||
if [[ -r /proc/sys/kernel/random/uuid ]]; then
|
||||
output_delimiter="CODEX_OUTPUT_$(cat /proc/sys/kernel/random/uuid)"
|
||||
else
|
||||
while :; do
|
||||
output_delimiter="CODEX_OUTPUT_$(mktemp -u XXXXXXXXXXXXXXXX)"
|
||||
|
||||
if ! grep -qxF "$output_delimiter" "$codex_output"; then
|
||||
|
Ghost marked this conversation as resolved
Outdated
gitea-actions
commented
嚴重等級:🔴 嚴重 **嚴重等級**:🔴 嚴重
**審查員**:Mage
**問題**:在 GitHub Actions 中寫入 `GITHUB_OUTPUT` 使用了動態分隔符(delimiter)。雖然邏輯正確,但若 `codex_output` 內容中恰巧包含了隨機生成的 `output_delimiter` 字串,將會導致輸出截斷或格式損壞。
**建議**:應先掃描 `codex_output` 內容,確保隨機分隔符字串不會出現在內容中,若有衝突則應重新生成分隔符。
|
||||
break
|
||||
|
Ghost marked this conversation as resolved
Outdated
gitea-actions
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Bard
**問題**:為了產生分隔符號而使用巢狀判斷來檢查 UUID 檔案,這讓原本流暢的腳本邏輯變得破碎,閱讀時節奏感不佳。
**建議**:建議直接統一使用 `mktemp -u` 產生隨機字串,捨棄繁瑣的 `if` 判斷,讓程式碼的旋律更輕快。
|
||||
fi
|
||||
done
|
||||
|
||||
if [[ "$codex_status" -eq 0 ]]; then
|
||||
echo "status=completed" >> "$GITHUB_OUTPUT"
|
||||
|
||||
嚴重等級:🟡 警告
審查員:Maya
問題:MODEL 為必填參數,但缺少針對 MODEL 為空字串或未定義時的行為測試。
建議:應在測試案例中模擬空 MODEL 輸入,並驗證腳本是否正確拋出錯誤並以 exit 1 終止。
嚴重等級:🟡 警告
審查員:Bard
問題:清理函式
cleanup內的變數命名 (auth_file,auth_path,codex_output,auth_lock) 雖清楚,但寫法稍顯瑣碎,且trap的慣用語法建議確保在變數未定義時也能安全執行。建議:在 Shell 腳本中,建議統一使用
unset或更簡潔的方式管理環境變數清理,並確保變數引用更具彈性。