# Codex Adversarial Review (4回目) — 603a49f 検証 - 対象: `~/projects/insta-auto-v3/packages/` (base=d5dcadb → HEAD=603a49f, branch=v3) - 実行日: 2026-04-17 - モデル: Codex (openai-codex plugin 1.0.3) via `/codex:adversarial-review` - 入力: `11_v3_Codex3回目レビュー_d5dcadb検証.md`（Codex 3回目レビュー指摘5件） - 目的: CC が対応した「Codex 3回目指摘5件」が実際に race / memory / chain 整合を解消できているかの検証 --- Target: branch diff against d5dcadb Verdict: **needs-attention** **No-ship.** Finding 1 は実質的に修正済み、Finding 2 の POST claim は single-use 化できた。しかし Finding 3 は不完全、Finding 4 は concurrency 下で依然 broken かつ migration 側に新規欠陥、Finding 5 は依然 PBKDF2 fan-out 脆弱。**子安氏への scope handoff は現状では不可**。 --- ## 🟢 材料的に修正されたと認定された項目 - **Finding 1 (handleFailure race)**: UPDATE...RETURNING 単一ステートメント化により解消 - **Finding 2 の POST 側 (承認 claim 原子的 NULL化)**: confirm_token が single-use 化 ## 🔴 依然残存する問題 ### [high] Finding 4a: audit seq 割当がハッシュチェーンのエッジを serialize しない **場所**: `packages/worker/src/services/audit.ts:93-96` `writeAuditLog` は 1ステートメントで seq を reserve した後、**別のステートメント**で直前 audit 行を SELECT し、さらに後で INSERT する。 2つの並行呼び出しで seq=1 と seq=2 を reserve した直後、seq=2 の writer が **seq=1 の INSERT より先に** SELECT を実行できてしまう。結果、seq=2 のエントリが `prev_hash=NULL`（または古い行のhash）でハッシュ化される。 **通常の並行audit トラフィックで chain が壊れる / false tamper signal を出す**。SQLite/D1 は個別 write は serialize するが、この multi-statement critical section は serialize しない。Codex 3回目で指摘した fork 問題は修正できていない。 **Recommendation**: - seq 割当 / prev-hash SELECT / INSERT / last-hash 更新 を **1つの serialized 操作** にする - 例: audit_sequence 1行をロックする transaction/batch - または `audit_sequence.last_hash` カラムを持たせ、新しい row hash を計算した後で atomic に更新 - 代替: audit 書き込みを Durable Object / Queue 経由にする --- ### [high] Finding 4b（新規）: 0005 migration が seq を backfill するが next_seq=1 のまま残す **場所**: `packages/worker/migrations/0005_v3_audit_chain_hardening.sql:20-23` 0005 は `audit_sequence.next_seq=1` を先に挿入してから既存 audit_logs を `seq=rowid` で backfill するが、**backfill後に next_seq を MAX(seq)+1 に進めていない**。 既存 audit_logs がある環境（本番）では migration 後の最初の audit 書き込みが: 1. 再び seq=1 を reserve する 2. `WHERE seq < 1` で prev_hash を SELECT するため「該当なし」 3. genesis から **2本目のチェーン** を開始してしまう さらに `audit_logs.seq` に **UNIQUE / NOT NULL 制約がない** ため、この重複が DB レベルで fail closed しない。 **Recommendation**: - backfill 後に `UPDATE audit_sequence SET next_seq = COALESCE((SELECT MAX(seq) FROM audit_logs), 0) + 1` - `audit_logs.seq` に UNIQUE index を追加 - **既存 audit_logs 行がある DB で migration 検証する**（空DBだけで検証は不十分） --- ### [high] Finding 5: login limiter は依然並列 PBKDF2 fan-out が可能 **場所**: `packages/worker/src/routes/auth.ts:47-72` reserve は依然 **2つのKV読み取り → 2つのKV書き込み**。同じIPからの同時ログイン試行バーストは全員が `perStoreAttempts/globalAttempts < limit` を観測し、全員が同じ reserved value を書き込み、全員が `verifyPassword` に進む。**PBKDF2 書き込みを前に倒しても、staggered attack を緩和するだけで atomic admission gate にはならない**。 加えて success release は `Math.max(0, reservedX - 1)` を書き戻すが、これは **他の in-flight failure が増分した値を上書きする** 可能性がある（正当ログインが他の攻撃者の punishment を消す）。 **Recommendation**: - PBKDF2 前に atomic counter（D1 `INSERT/UPDATE ... RETURNING` の transaction、または Durable Object） - success release を「atomic conditional update」に変更（locally-computed stale 値を write しない） - または **本項目は fixed と marking せず、子安氏スコープ（A6: Durable Objects）に明示的に残す** --- ### [medium] Finding 3: Upload は enforceable limit 前にバッファリングしている **場所**: `packages/worker/src/routes/upload.ts:40-52` 唯一の pre-parse ガードは **optional な** Content-Length。ヘッダが欠落・不正確な場合、file count / file size / totalBytes チェック前に `c.req.formData()` が呼ばれ、**20MB cap を超える multipart body が Worker 内にバッファされる**。 accepted な 20MB バッチでも以下が累積する: - parsed File bodies - per-file ArrayBuffer - binary string - base64 data URL（uploaded 配列内） - JSON response Sequential処理は Promise.all amplification を避けるが、**敵対的 or 並行 upload に対して memory-safe ではない**。 **Recommendation**: - Content-Length 不在時は **fail closed**（この endpoint が stream できない場合） - または `formData()` を hard byte counter付き streaming multipart parser に置換 - レスポンスには **stored object ID/URL を返す**（base64 data URL を embed しない） - R2 / streamed storage が durable な fix --- ## Next steps（Codex推奨） - **CC は audit serialization と 0005 migration を handoff 前に修正すること** - **CC は KV login limiter を atomic admission gate に置換するか、Finding 5 を子安氏スコープとして明示的にオープンのままにする**（"fixed" と marking してはいけない） - **Upload handling を「body 読み取り中」にサイズ enforce するよう再設計**（formData() 後の enforce は不十分） --- ## 結論とCC側のアクション可否 | # | 問題 | CC対応可否 | 子安氏スコープに残す？ | |---|---|---|---| | 4a | audit_sequence last_hash を同テーブル内に持たせて atomic 化 | ✅可能 | 否（CCで直せる） | | 4b | 0005 migration に next_seq 前進 + UNIQUE index 追加 | ✅可能 | 否（CCで直せる） | | 5 | 「fixed」marking を撤回し A6 にラベル移動 | ✅可能 | **はい（Durable Objects 必須）** | | 3 | Upload レスポンスから base64 を外し object ID だけ返す | ⚠️ API 変更必要・影響範囲大 | **はい（R2 と同時が合理的）** | **推奨アクション**: - 4a, 4b, 5（markingのみ撤回）を CC で追加修正 - 3 は R2 移行と抱合せで子安氏スコープ（A2）に明示的に残す 合計追加工数: 3〜5h（audit serialization 1時間 + migration fix 30分 + auth.ts markingコメント修正 30分 + VERIFY.sh 更新 1〜2h）