Day 12: Code Review 工作流：建立 AI 重構 (Refactoring) 與資安掃描機制 (AI Code Review and Security Scan)

本日核心價值 (Core Focus): 用固定檢核清單與嚴重度表，讓 Codex /review 做正確性、注入風險、secrets、錯誤處理與測試缺口的防禦性審查，並把規則從 AGENTS.md 指向 REVIEW.md。

概念說明與實戰情境 (Overview)

測試綠燈只代表「目前案例通過」，不代表 diff 可合併。字串拼接 SQL、把密碼寫進設定檔、吞掉例外、缺少回歸測試，都不會被 pytest 自動抓到。Code Review 工作流要把審查變成結構化輸出：每筆 finding 有檔案、符號、嚴重度、證據、建議修法。Codex 提供 /review（對 base branch、未提交變更或指定 commit），GitHub 也可掛自動 PR review。官方建議：AGENTS.md 保持精簡，審查細則放到獨立檔並引用；本系列用 REVIEW.md（對應文件示例中的 code_review.md）。本日只做防禦性審查與參數化重構，不提供攻擊 PoC 或利用步驟。AI 審查不能取代人對授權邏輯的最終判斷，但可以把「明顯危險的組裝查詢」從 PR 裡攔下來。

關鍵操作與範例 (Implementation & Example)

先在 repo 根目錄放兩份短檔，讓每次 review 讀同一套閘道，而不是每次重貼長 Prompt。

# AGENTS.md（節錄）

## Review
Follow REVIEW.md for all code review and security-oriented refactors.
Do not produce exploit payloads or attack reproduction steps.
Prefer parameterized queries and explicit error handling.
After review, only patch findings the human accepted.

# REVIEW.md

Checklist (must classify each finding):
1. Correctness: wrong branch, off-by-one, broken invariant vs existing tests.
2. Injection: SQL or command built by concatenating untrusted input.
3. Secrets: API keys, connection strings, tokens hard-coded or logged.
4. Error handling: swallowed exceptions, empty catch, leaking internals to clients.
5. Tests: missing regression for the changed behavior.

Output a table: Severity (Critical / High / Medium), File, Symbol, Evidence, Fix.
Critical: secret in repo or untrusted input reaches SQL/command construction.
High: incorrect money/authz logic, empty catch around persistence.
Medium: missing tests, unclear error mapping.
Do not invent Critical without a cited code path.

審查 Prompt 要求表格，避免散文式「看起來還好」：

Review the current diff against main using REVIEW.md.
Scope: correctness, SQL/command injection smells, secrets, error handling, tests.
Output ONLY:
1) Markdown table with columns: Severity, File, Line, Issue, Evidence, Defensive fix.
   Severity must be Critical, High, or Medium.
2) For each SQL concatenation smell, show the parameterized replacement in the same language.
3) List checklist items with no finding as "Clear".
Do not write exploit PoCs, payloads, or injection strings.
Do not change files in this pass; read-only review.

互動式可用 /review，並加上「Focus on REVIEW.md checklist」。非互動、且本趟只讀不改時，可用 read-only；若審查過程要寫報告檔到 workspace，再改 workspace-write，但 Prompt 仍應寫 Do not modify application source。

codex exec --sandbox read-only --ask-for-approval never \
  "Review uncommitted changes using REVIEW.md. Emit the severity table. Do not edit source."

以下是常見的 C# 味道：把使用者可影響的值拼進 SQL。審查應標 Critical 或 High（視輸入是否外來），並改成參數化。不要在文章或模型輸出裡示範如何繞過。

// 味道：查詢字串與資料混在一起，審查應要求參數化
public async Task<Order?> GetOrderAsync(string userId, string orderId)
{
    var sql = "SELECT Id, Total FROM Orders WHERE UserId = '" + userId
              + "' AND Id = '" + orderId + "'";
    await using var cmd = new SqlCommand(sql, _connection);
    await using var reader = await cmd.ExecuteReaderAsync();
    // ...
}

// 防禦性重構：語意固定在程式，值只走參數
public async Task<Order?> GetOrderAsync(string userId, string orderId)
{
    const string sql = """
        SELECT Id, Total
        FROM Orders
        WHERE UserId = @userId AND Id = @orderId
        """;
    await using var cmd = new SqlCommand(sql, _connection);
    cmd.Parameters.Add("@userId", SqlDbType.NVarChar, 64).Value = userId;
    cmd.Parameters.Add("@orderId", SqlDbType.NVarChar, 64).Value = orderId;
    await using var reader = await cmd.ExecuteReaderAsync();
    if (!await reader.ReadAsync())
    {
        return null;
    }
    return new Order(
        reader.GetString(0),
        reader.GetDecimal(1));
}

Python 對等修法是參數佔位，而不是 f-string 組 SQL：

# 味道
cur.execute(f"SELECT id, total FROM orders WHERE user_id = '{user_id}'")

# 防禦性重構（psycopg / DB-API 風格）
cur.execute(
    "SELECT id, total FROM orders WHERE user_id = %s",
    (user_id,),
)

Command 注入同樣只描述味道與修法：subprocess 若把未信任字串交給 shell，應改成參數陣列、不經過 shell。不要在 finding 裡附「成功執行外來命令」的證明。

Secrets：設定檔或原始碼出現 Password=、sk-、雲端金鑰，標 Critical，修法是改環境變數或 secret store，並視為需輪替的憑證；審查輸出寫「已硬編碼、應移出 repo」，不要把完整密鑰再抄進表格。

錯誤處理：catch (Exception) { } 或 except Exception: pass 若包住寫入或授權，至少 High。測試缺口：Day 11 的金額進位若這次改了捨入卻沒更新 pytest，標 Medium 並要求補回歸。

模型產出應接近：

通過後才做 Refactoring：一次只收人類接受的 finding，避免「順手重寫資料層」。Reviewer 角色在 Day 15 會固定為 read-only；本日先養成「先表、後補丁」的習慣。

/review 的範圍要選對：對未提交變更可抓本地拼接 SQL；對 base branch 則適合 PR。自訂指示只要一句「Follow REVIEW.md；output Critical/High/Medium table」。官方也允許把細則放在獨立檔並從 AGENTS.md 引用；本系列檔名用 REVIEW.md，與文件示例的 code_review.md 同角色。

Command 注入的防禦描述同樣停在修法：把未信任字串拼進 shell 命令列，應改成參數陣列、不啟動 shell，並拒絕把使用者輸入當執行檔名。審查表格寫「此處把輸入拼進命令」，不要附可執行的命令列證明。Secrets 輪替是運維動作，模型輸出只提醒「此檔含憑證樣式、應移出並視為已洩漏」，不要把金鑰全文複製到 finding。

人類勾選 finding 時建議只收 Critical 與 High 進同一張修補 PR；Medium 的缺測可以跟 Day 11 的測試閉環併單。這樣 Reviewer 不會在下一輪又看到「順便重寫 repository」。參數化補丁本身也要過測試：至少斷言 SQL 字串是常數、參數集合含 @userId，而不是只靠 code review 表格當通過證明。

注意事項與常見失敗 (Pitfalls)

• 把 review 寫成攻擊教學：輸出 payload 或逐步利用。修法：REVIEW.md 與 Prompt 都寫明只做防禦性修法；違反就當失敗輸出重跑。
• 嚴重度通膨：風格問題標 Critical，真正的拼接 SQL 反而埋在段落裡。修法：沒有引用程式路徑不得標 Critical。
• AddWithValue 造成隱性型別轉換：看似參數化仍可能走錯查詢計畫。修法：明確 SqlDbType 與長度。
• 只跑 /review 不引用 REVIEW.md：每次清單不同，團隊無法對齊。修法：AGENTS.md 指向 REVIEW.md。
• 審查與修復同一趟、同一 sandbox：模型可能邊看邊改，finding 與 diff 對不上。修法：先 read-only 出表，人類勾選後再 workspace-write 小補丁。

本日總結 (Takeaways)

• 檢核固定五項：正確性、SQL/command 注入味道、secrets、錯誤處理、測試。
• 輸出必須是 Critical / High / Medium 表，每列都有檔案與證據。
• 字串拼接 SQL 的修法是參數化，不是「過濾特殊字元」的口頭保證。
• 官方 /review + AGENTS.md 引用獨立審查檔；本系列該檔名為 REVIEW.md。
• 不寫 exploit PoC；修復與審查分兩趟，避免邊審邊改。
• Critical 必須能指出「未信任輸入走進查詢或命令組裝」或「密鑰在檔案裡」；做不到就降級，不要為了表格好看而升級。

明日預告 (Next)

明天進入快速 Debug 工作流：結合 Error Logs 與 AI 自動對齊 Stack Trace 定位問題，把例外從「整份 log」收成可驗證的最小重現。