# Codex Adversarial Review (3回目) — d5dcadb 検証

- 対象: `~/projects/insta-auto-v3/packages/` (base=10ecb30 → HEAD=d5dcadb, branch=v3)
- 実行日: 2026-04-17
- モデル: Codex (openai-codex plugin 1.0.3) via `/codex:adversarial-review`
- Turn ID: 019d9e87... (foreground)
- 目的: CC が対応した「Codex再レビュー(09/10)指摘 CC担当分」が実際に race / side-effect / memory を解消できているかの検証

---

Target: branch diff against 10ecb30
Verdict: **needs-attention**

**No ship on the CC-scope fixes.** The scheduler claim predicate fix is real, and password/keyring/CORS basics look improved, but the failure path, approval token model, upload cap, audit chain, and login limiter still have **material failure modes**.

---

## Findings

### [high] Failure path releases the scheduler lock before setting backoff
**場所**: `packages/worker/src/services/scheduler.ts:206-262` (handleFailure)

`handleFailure` は retry_count を increment し、`processing_started_at / locked_by` を先にクリアした後、別ステートメントで retry_count を SELECT し、さらに別の UPDATE で status / next_attempt_at を設定する。このギャップ中、行は **status='scheduled' / unlocked / due / next_attempt_at 未設定（or NULL）** となり、retry_count < MAX のケースで他 Cron が claim 述語を満たして再びpublishできてしまう。**failure path で next_attempt_at race が再登場** し、リトライを消費 or Instagram に重複投稿する可能性。

**Recommendation**: retry_count increment / next_attempt_at or status 決定 / last_error / ロック解放 を **1 つの条件付き UPDATE** にまとめる（RETURNING で post-increment を取得）。RETURNING が使えない場合は最終 state 更新がコミットされるまで `locked_by` を保持する。

---

### [high] Confirm token is minted by the same leaked approval URL
**場所**: `packages/worker/src/routes/approval.ts:54-117`

GET `/approval/:token` は認証も status 条件もなしに「期限内の approval token」であれば **誰でも新しい confirm_token を発行・保存できる**。URL を持つ任意の相手が GET → POST の2段で副作用を起こせる。さらに POST 処理後に confirm_token は NULL 化されないため、失敗時の pending 復帰後も同じ confirm_token で再利用可能（**single-use ではない**）。bearer-link side-effect が「2リクエスト bearer-link side-effect」に変わっただけ。

**Recommendation**: 
- 未認証 GET で action token を発行しない
- 発行条件に `status='pending'` を必須化
- 承認者セッション（HttpOnly same-site cookie）で confirmation を縛る
- pending→processing UPDATE 内で confirm_token を **原子的に NULL化**
- GET にもレート制限を掛ける

---

### [high] 60 MB upload cap still exceeds Worker memory after buffering
**場所**: `packages/worker/src/routes/upload.ts:28-75`

`formData()` parse を先に実行してから cap を enforcement している。そこから `Promise.all` で全ファイルを並行バッファリング。**60MB バッチ** でも isolate 内には以下が同時に保持される:
- multipart File データ
- 60MB ArrayBuffers
- 大きな binary string（`String.fromCharCode`）
- 約 80MB の base64 / data URL
- JSON response buffer

128MB isolate 上限に対して **安全マージンではない**。memory exhaustion 攻撃経路が残る。

**Recommendation**:
- `formData()` 呼び出し前に **Content-Length で拒否**
- ファイルを sequential 処理（Promise.all を廃止）または直接 R2/KV にストリーム
- Worker から base64 data URL を返さない（URL のみ返す）
- この buffering 設計のまま続ける場合は cap を大幅に下げる（例: 20MB）

---

### [medium] Audit hash chain can fork under normal write ordering
**場所**: `packages/worker/src/services/audit.ts:63-96`

`writeAuditLog` は `ORDER BY ts DESC LIMIT 1` で prev_hash を SELECT するが:
1. **ts は秒単位に truncate** されている（同秒内の順序が不定）
2. **read / compute / insert がシリアライズされない** - 並行書込で同じ prev_hash を読んで兄弟エントリが生まれる
3. `canonicalize()` は **トップレベルのみ sort**、metadata は pre-stringified なので **ネストしたオブジェクトの順序は正規化されない**

honest load でもチェーンが分岐・フォークし、**改竄検知の前にチェーンが破綻**する。

**Recommendation**:
- transactionally-updated head row または CAS 方式を使う
- monotonic sequence（単調増加シーケンス）で順序を保証
- タイムスタンプはマイクロ秒 or 高精度に
- metadata を deep-canonicalize、または保存される metadata_json の exact バイト列をハッシュ
- tamper evidence が DB 書換アクセスを超えて必要なら外部アンカリング（blockchain anchor, external log）

---

### [medium] Global login rate limit loses updates under parallel storeId rotation
**場所**: `packages/worker/src/routes/auth.ts:32-80`

per-store / global カウンタを read once → 遅延 write し、stale value + 1 で上書きする。異なる storeId への並行失敗バーストでは全員が `globalAttempts=0` を観測、結果として global key は `1` に上書きされ続ける。**global カウンタが warm up しない**。
さらに、PBKDF2 検証が失敗カウンタ increment より前に走るため、**高コストな hash 計算が race 下で fan-out** する（CPU消費攻撃経路）。

**Recommendation**:
- atomic counter または Durable Object lease / D1 transaction を使う
- 可能なら PBKDF2 検証の **前に** 失敗予約 increment
- WAF / Turnstile を外層の abuse control として保持

---

## Next steps

### 🔴 前回指摘が「still broken or only partially fixed」と判定された項目
- **Scheduler handleFailure の retry race** — 依然として存在
- **Approval bearer-token risk** — GET で confirm_token 発行可能なので実質 bearer のまま
- **Audit hash-chain reliability** — fork 可能
- **Login rate limit under concurrency** — race で warm up しない
- **Upload memory safety** — 60MB 上限でも 128MB isolate には危険

### 🟢 「materially improved」と判定された項目
- Scheduler claim UPDATE に eligibility 述語が含まれる（1.1 の前半は OK）
- PBKDF2 versioning / needsRehash は正しく実装されている
- crypto / JWT primitives が未知 version / kid を正しく拒否する
- 未知 CORS origin が localhost echo fallback を受けない

### ⚪ 子安氏スコープとして残置が期待通り
- idempotent publish recovery
- R2-backed media storage
- Cloudflare Queues
- WAF / Turnstile
- 認証済み承認者フロー

---

## 結論とアクション

**ship-ready ではない**。CC対応の 5項目のうち「方向性は正しいが実装に瑕疵あり」が 5件残っている。

子安氏レビュー前に **追加で CC 側が対応すべき項目** が発生:

| # | Codex指摘 | CC対応可否 | 推定工数 |
|---|---|---|---|
| 1 | handleFailure を単一の条件付き UPDATE に統合（RETURNING 使用） | ✅可能 | 1〜2h |
| 2 | 承認 GET で confirm_token 発行を status='pending' に限定 + POST時に NULL化 + GET側のrate limit | ✅可能 | 2〜3h |
| 3 | Upload: Content-Length 事前拒否 + sequential 処理 + cap 下げる（20MB） | ✅可能 | 1〜2h |
| 4 | Audit chain: micro-second timestamp + monotonic sequence カラム + deep canonicalize | ✅可能 | 2〜4h |
| 5 | Rate limit: read→write race の縮小（可能なら Durable Object 検討、CC可能範囲では PBKDF2 前 reserve） | 🟡 部分可 | 1〜2h（atomic化は子安氏へ） |

**合計: 7〜13h（約1〜2人日）で CC側の2次修正が完了可能**。
