test(architecture): 規約違反を機械で検出し, 機械化した分を Skill から削る

[ttokoro20240902]

ドラフトです。 3 本とも動作・検出力は確認済みですが、検査対象をさらに増やす予定があり、テストの置き場（Architecture/ の新設）についても意見を伺いたいためドラフトにしています。

概要(Overview・Refs Issue)

AI 向けレイヤ規約（.claude/skills）は description による自動発火に依存するため、必ず読まれるとは限りません。実測では、実際の Issue に対応させたときレイヤ規約への到達は 6 割程度で、残りは規約を読まないまま実装が進みます。

読まれなくても守られる分を機械検査で確保します。 人が書いた場合にも同じく効きます。

方針(Policy)

規約 152 項を機械検出の可否で分類すると、具体的な API 名・パスで書かれた 78 項（51%）が検出可能でした。そのうち実害が大きく、偽陽性をゼロにできるものから着手します。

追加する検査

#	テスト	検出内容	現状の違反
1	AdminRoutePrefixTest	管理コントローラのルートが %eccube_admin_route% 配下にあるか	0 / 187
2	CsrfProtectionTest	GET 以外だけを受けるアクションが CSRF から保護されているか	0 / 74
3	SkillFrontmatterTest	Skill の description が YAML として壊れていないか	1 件（本 PR で修正）

1・2 は現状 0 違反なので、既存コードを直す必要がなく、回帰防止のゲートとして機能します。

なぜこの 3 つか

いずれも画面上は正常に動いてしまい、気づく契機が無い種類の問題です。

• 1 — 管理画面の認可は admin ファイアウォール（pattern: ^/%eccube_admin_route%/）が担うため、配下から外すと未認証で到達できます。画面は動くのでレビューでも見落としやすい。
• 2 — CSRF 検証の漏れも動作に影響しません。
• 3 — description はクォートされないプレーンスカラーなので、: （コロン+空白）や #（空白+シャープ）で以降が捨てられます。一覧に載る説明が欠け、失われたトリガ語では自動発火しなくなります。 本文は Markdown として普通に読めるため気づけません。実際に eccube-migration が壊れていたので併せて修正しました。

2 つ目のコミット: CI が機械化した分を Skill から削る

CI を足す目的は Skill を守ることではなく、実コードを守り、その分 Skill を軽くすることです。 機械が落とせるものを Skill に残すと二重管理になり、1 Skill 10 項の枠を食って本当に必要な項を書けなくします。

本 PR の CsrfProtectionTest が機械で検査するようになったため、同じ内容を指示していた 2 項を削除しました。

Skill	削除した項	項数
eccube-controller	削除/Ajax 等の状態変更でトークン未検証	9 → 8
eccube-security	フォームを介さない POST/DELETE/Ajax で CSRF 未検証	10 → 9

次の 1 項は機械化されていないため残しました。

eccube-controller — 戻り値を捨てた isTokenValid(); を「CSRF 未検証」と誤読

isTokenValid() は失敗時に AccessDeniedHttpException を投げるため、戻り値を捨てた bare 呼び出しでも検証は成立します（AbstractController の実装で確認）。呼び出しの有無ではなく読み手の誤解を正す項なので、検査では代替できません。

あわせて AGENTS.md の歯止め（1 項 120 字）を超えていた 6 項を、内容を落とさず短縮しました（最大 132 字 → 119 字）。

偽陽性の除去

CsrfProtectionTest は単純に書くと偽陽性が 14 件出ました。次を正当な例外として除外しています。

除外	理由
AgentCommerce/ Mcp/	外部エージェント向け API。ブラウザセッションを使わないので CSRF 対象外
isXmlHttpRequest()	XHR 限定（ブラウザからの単純な form POST を弾く）
同クラス内の別アクションへの委譲	委譲先で検証される（LayoutController::preview → edit()）

偽陽性が 1 件でもあると検査ごと無効化されてしまうため、0 件にしてから提出しています。

CI での実行

DB もカーネルも使わない静的な検査なので、PHP × DB のマトリクス（16 セル）で走らせる意味がありません。

• メインの実行に --exclude-group architecture を追加
• 専用ジョブ 1 セルのみ（PHP 8.2、DB なし、composer install --no-scripts）

既存の cache-clear / mcp / plugin-service / rector など 10 個のグループと同じ分離方式です。

実装に関する補足(Appendix)

• 検出力は 3 本とも確認済みです。OrderController::bulkDelete から isTokenValid() を外す、AdminController のルートから %eccube_admin_route% を外す、といった違反を仕込んで落ちることを確かめ、復元して緑に戻しています。
• eccube-migration の 1 行修正は docs(skills): レイヤ規約 description の対象範囲にプラグインを含める #7106 にも含まれます。マージ順によってはこの 1 行がコンフリクトします。
• 検査は 152 項のうち 3 項をカバーします。残りの候補（Ajax の XHR 限定、fgetcsv/fputcsv の直書き、Twig の |raw、Skill の項数・字数上限）は別 PR で順次追加する想定です。

テスト（Test)

vendor/bin/phpunit --group architecture
OK (168 tests, 428 assertions)

• PHPStan（level 6）: エラーなし
• PHP-CS-Fixer: 修正なし
• メインの実行から除外されることを --exclude-group architecture --list-tests で確認（該当 0 件）

相談（Discussion）

• テストの置き場を tests/Eccube/Tests/Architecture/ と tests/Eccube/Tests/Skill/ に新設しました。既存は src/Eccube/ の構造に対応した命名（Controller / Doctrine / Repository …）なので、機能別ディレクトリに寄せるべきかご意見を伺いたいです。test(entity): if(!class_exists()) ガードの再発防止ゲートを追加 (refs #6891) #7113 は Doctrine/ORM/Mapping/ に置いています。
• SkillFrontmatterTest は .claude/skills/ を検査するもので、src/Eccube/ のアーキテクチャとは性質が違います。PHPUnit でやるべきか（専用スクリプト + CI ステップにする案もあります）も論点です。

マイナーバージョン互換性保持のための制限事項チェックリスト

• 既存機能の仕様変更はありません
• フックポイントの呼び出しタイミングの変更はありません
• フックポイントのパラメータの削除・データ型の変更はありません
• twigファイルに渡しているパラメータの削除・データ型の変更はありません
• Serviceクラスの公開関数の、引数の削除・データ型の変更はありません
• 入出力ファイル(CSVなど)のフォーマット変更はありません

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[ttokoro20240902]

内容を #7115 へ統合したため close します。

全レイヤの「よくある間違い」を eccube-pre-impl の 1 ファイルへ集約する構成変更を入れたところ、
Skill 関連の PR が分かれたままでは相互に衝突し、マージ順に依存する後追い作業が発生する状態になりました。
このため #7115 に 1 本化しています。

本 PR のコミットは #7115 にマージコミットとして取り込んであり、内容は失われていません。
統合にあたっての調整点は #7115 の本文に記載しています。