feat(agent-commerce): UCP checkout (#6574) のコア実装#6837
Open
nanasess wants to merge 32 commits into
Open
Conversation
ACP/UCP 対応の共通基盤のうち CheckoutSession 非依存の先行スライスを実装。 トラック A (Product Feed / Discovery) を解放する最小集合。 - MinorUnitConverter: 通貨 minor unit 変換 (bcmath / ゼロデシマル / 負数) - AddressMappingService: 住所マッピング・国コード numeric→alpha-2 (全249件網羅) - AgentCommerceScopeRegistry: <protocol>:<capability> scope 照合 - KeyStoreInterface / FilesystemKeyStore: 鍵保管 (env パス上書き→既定ファイル、EC-CUBE#6797 雛形) - AgentCommerceMessageSignerInterface / UcpMessageSigner: RFC 9421 EC P-256 / JWK 公開鍵 / 鍵ローテーション grace - BaseInfo にフラグ5 (acp/ucp 有効化等、default false) + google_pay_merchant_id + migration - 秘密鍵は dtb_baseinfo でなく app/keystore/ に保管 (EC-CUBE#6797、.htaccess/.gitignore 多重防御) プライバシー/利用規約 URL はカラム化せず標準ページ (help_privacy/help_agreement) から自動生成する方針。 検証: PHPUnit 53 tests / 611 assertions / 0 失敗、PHPStan level6 No errors、php-cs-fixer 0件。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
minor-unit は通貨の小数桁数だけ桁が増える (一律 ×100 ではない) ことが 一目で分かるよう、docblock の例を JPY (×1) / USD (×100) の 2 桁までに統一。 3 桁通貨 (BHD 等) の 4 桁例は紛らわしいため削除。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ACP/UCP 用の 6 カラムは BaseInfo エンティティに #[ORM\Column] で定義済みのため、 公式アップデート手順 (doctrine:schema:update --force) で自動反映される。 カラム追加に ALTER TABLE マイグレーションを書かないのが EC-CUBE の慣例 (前例 PR EC-CUBE#4912: カラム追加に ALTER マイグレーション無し、INSERT のみ)。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI の rector ジョブ (PR EC-CUBE#6802) で検出された 8 ファイルの指摘を vendor/bin/rector で適用。 - AddressMappingService: 冗長な (int) キャストと三項を null 合体演算子に簡約 - UcpMessageSigner: コンストラクタプロパティ昇格 - テスト各種: self:: → $this->、final class 化、strict_types 宣言等 AgentCommerce テスト 53 件すべて成功を確認。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
決済はプラグイン化方針 (Stripe 同様) のため、Google Pay の merchant_id は core BaseInfo に持たせない。UCP discovery の payment_handlers は決済ハンドラ プラグインが寄与する設計とする。あわせて rector 適用後の php-cs-fixer 整形 (ライセンスヘッダ直後の空行) をテストへ反映。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AddressMappingService の numeric->alpha-2 ハードコード const (249件) を廃止し、 新規マスタ mtb_country_iso_code で管理する。PR EC-CUBE#6802 レビュー指摘 (マスタテーブル化) に対応。 - 新規マスタ CountryIsoCode / CountryIsoCodeRepository を追加 - mtb_* の固定スキーマ (id/name/sort_no/discriminator_type) に準拠し、id=ISO numeric / name=alpha-2 を格納 (discriminator=countryisocode) - 新規インストールは import_csv (definition.yml 登録)、既存環境は INSERT データ migration で backfill (mtb_country は改変しない) - AddressMappingService はリポジトリ経由解決へ変更、テストを Layer 2 化 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CI rector ジョブ (ContainerGetNameToTypeInTestsRector / AssertFuncCallToPHPUnitAssertRector)
の指摘に対応。get('doctrine') + ManagerRegistry 手動構築をやめ、services_test.yaml で
AddressMappingService を public 化しコンテナから FQCN 取得する方式へ変更。
- services_test.yaml: AddressMappingService を public 化 (consumer 未実装で private では除去されるため)
- AddressMappingServiceTest: self::getContainer()->get(AddressMappingService::class) に簡素化
検証: PHPUnit 52/608、PHPStan level6 No errors、php-cs-fixer 0、rector dry-run クリーン。
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
BaseInfo の有効化フラグを「checkout の有無を制御する」意味へ整理する。 discovery / catalog は公開して害がないため常時公開とし (ゲート撤去は EC-CUBE#6794)、 ACP feed push は認証情報の有無で実質ガードされるためフラグ不要とする。 - 改名: acp_enabled → acp_checkout_enabled / ucp_enabled → ucp_checkout_enabled (getter は isAcpCheckoutEnabled / isUcpCheckoutEnabled) - 削除: acp_feed_enabled (push は base URL + API key の有無でガード) / ucp_catalog_api_enabled (UCP Catalog は常時公開) - 維持: ucp_catalog_requires_auth (catalog の OAuth 必須モードを api4 着手時に実装) - dtb_base_info.csv (ja/en) ヘッダを 3 フラグへ更新 - 店舗設定 (ShopMasterType / @admin/Setting/Shop/shop_master.twig) に acp_checkout_enabled / ucp_checkout_enabled のトグルを追加 (checkout は日本未提供の注記つき) - BaseInfoAgentCommerceFlagsTest を 3 フラグへ更新 PHPStan level 6 No errors / php-cs-fixer 0 / 関連テスト green。 カラム変更は schema:update 方式 (ALTER マイグレーションは書かない)。 Refs EC-CUBE#6777 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- MinorUnitConverter: 不正な金額文字列 (abc / 1,000 / 1..2 等) を BCMath 呼び出し前に正規表現で弾き、ValueError を防止 (仕様の「不正は 0」契約遵守) - FilesystemKeyStore: 鍵書き込み時の mkdir / file_put_contents 失敗を RuntimeException で検知し、サイレント失敗を防止 - Version20260604120000: down() を不可逆 migration として明示し、 新規インストール (up() が no-op) 環境での import_csv 由来データ巻き添え削除を回避 - AddressMappingServiceTest: fopen 後に assertIsResource を追加し診断性を向上 - MinorUnitConverterTest: 不正・空入力の回帰テスト 8 ケースを追加 CodeRabbit 指摘のうち COUNT(*) 判定と (bool) キャストの 2 件は非妥当のため非対応。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MinorUnitConverter::toMinorUnits の入力検証を、正規表現での形式チェックから bcmul/bcadd の ValueError 捕捉へ変更する。 - BCMath が「受け付ける数値文字列形式」の唯一の権威であり、正規表現で再実装すると 仕様の二重管理・他箇所への正規表現の拡散を招くため、判定を BCMath 自身へ委譲する - 空文字 '' / '.' は BCMath が 0 として扱うため明示チェックも不要になり簡素化 - is_numeric は指数表記 (1e3) を true と判定するが BCMath では ValueError になり 不適 (実証済み)。ValueError 捕捉なら指数表記・hex も正しく 0 に倒せる - 回帰テストに指数表記 (1e3 / 1.5e3) と hex (0x1A) ケースを追加 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
FilesystemKeyStore::write が file_put_contents で umask 既定 (通常 0644) の パーミッションでファイルを作成し、その後 chmod(0600) するため、その間だけ 秘密鍵が group/other から読める瞬間が生じていた。書き込みの間だけ umask(0077) に切り替え、作成時点から 0600 になるようにする。あわせて chmod の戻り値を検査し失敗時に例外を投げる。 CodeRabbit レビュー指摘 (PR EC-CUBE#6825 経由・本ファイルは EC-CUBE#6802 由来) 対応。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
エージェントコマース (ACP/UCP) checkout の前提となる、プロトコル非依存の CheckoutSession 中核を実装する。checkout エンドポイント本体は EC-CUBE#6776/EC-CUBE#6574 に委ね、 本コミットは再利用可能なエンティティ・サービス・認証部品とテストを提供する。 - CheckoutSession エンティティ + Repository を新規追加 (Cart をセッションレスに 束ねる上位層・status 正規化(canceled 含む)・json 列・Cart/Order/Customer 関連) - Order に agent_protocol / agent_id を NULL 許容で追加 (通常購入は両 NULL) - AgentCheckoutPurchaseFlowAdapter: 中立 DTO → Cart(永続) → OrderHelper → 既存 shopping flow 再利用で税・送料・在庫を再計算 (新規フローは作らない) - FulfillmentOptionMapper: 送料(DeliveryFee×Pref)・代引手数料(PaymentOption→ Payment::charge)・配送日数(DeliveryDuration の明細横断 max) を解決 - CustomerResolverInterface + GuestCustomerResolver (標準はゲスト=null。会員 ID 連携は eccube-api4#189 landing 後に差し替える seam) - AgentCommerceOAuth2Authenticator: Symfony 標準 AccessTokenHandlerInterface 経由で トークン検証 + scope×protocol 照合。eccube-api4 具象に依存せず、未導入時 503 - AgentCheckoutException + ErrorCode enum、AgentCheckoutPaymentHandlerInterface 共通基底 - カラム追加・新規テーブルは migration を書かず schema:update 方式 (既存規約に準拠) テスト: Layer 0 (仕様適合) / Layer 2 (Doctrine) / Layer 3 (PurchaseFlow 連携) / Layer 4a (OAuth2 ユニット・api4 不要)。AgentCommerce スイート 92 tests 0 失敗、 PHPStan level6 No errors。Layer 4b 統合は eccube-api4#188 landing 後。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ull 引数除去) CI の rector ジョブ指摘を反映: - DTO/値オブジェクトを readonly class 化 (ReadOnlyAnonymousClassRector ほか) - テストの self::assert* を $this->assert* に統一 (PreferPHPUnitThisCallRector) - null デフォルト引数への明示 null 渡しを除去 (RemoveNullArgOnNullDefaultParamRector) PHPStan level6 No errors / AgentCommerce 92 tests 0 失敗を維持。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CheckoutSession.status / protocol および Order.agent_protocol を、文字列カラム + クラス文字列定数から、Order/OrderStatus と同じ「マスタ + INTEGER ManyToOne」方式へ変更。 区分値を文字列で保存しない EC-CUBE コア慣習に統一する (protocol は CheckoutSession と Order の双方で参照するため特にマスタ化が必須)。 - 新規マスタ mtb_checkout_session_status (CheckoutSessionStatus・INCOMPLETE=1..EXPIRED=5) - 新規マスタ mtb_agent_protocol (AgentProtocol・ACP=1/UCP=2) - 各 Repository、import_csv (ja/en・name は正準値で同一)、definition.yml 登録 - 既存インストール向け backfill の INSERT migration (冪等・down 非対応) - CheckoutSession.status/protocol、Order.agent_protocol を ManyToOne 関連へ - AgentCheckoutRequest.protocol(string) → protocolId(int)、Adapter で AgentProtocol 解決 - テストをマスタ参照へ更新 (status()/PHPUnit final 衝突回避で findStatus にリネーム) PHPStan level6 No errors / AgentCommerce 93 tests 0 失敗 / Order・PurchaseFlow 回帰 green。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CheckoutSession が参照する Cart が、ログイン会員の Web カート解決 (CartService::getPersistedCarts は customer_id で全カートを取得) に混入し、 マージ・再計算・購入完了時の削除等で操作されてしまう問題を防ぐ。 Web 側の Cart 再生成に CheckoutSession を追従させるのは侵襲的で脆いため、 エージェント所有カートを Web から不可視・操作不可に隔離する方針とする。 - Cart に agent_owned (boolean, default false) を追加 - CartService::getPersistedCarts/getSessionCarts の検索条件に agent_owned=false を追加し、 エージェント所有カートを Web カート解決から除外 (既存カートは全て false で無影響) - Adapter はカートに agent_owned=true をセットし、customer_id は常に NULL に保つ (会員帰属は Order 側に持たせ、会員 ID 連携時もカート混入を防ぐ多層防御) - agent_owned カラム追加は schema:update 方式 (migration 不要) - 隔離の回帰テストを追加 PHPStan level6 No errors / AgentCommerce 85 tests 0 失敗 / CartService 回帰 green。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
expires_at と現在時刻が同値の瞬間だけ未失効になっていた `<` 判定を `<=` に変更し、境界を期限切れ扱いにする。境界値の回帰テストを追加。 CodeRabbit レビュー指摘 (PR EC-CUBE#6825) 対応。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
OAuth2 Authenticator テストのスタブ handler が、rector の ReadOnlyAnonymousClassRector により `new readonly class` (匿名 readonly クラス) へ変換されていた。匿名 readonly クラスは PHP 8.3+ 専用で、 サポート下限 (PHP 8.2) の CI で `syntax error, unexpected token "readonly"` となり PHPUnit が失敗していた。 名前付き `final readonly class ScopeStubAccessTokenHandler` (8.2 で有効・ rector の匿名クラスルール対象外) へ切り出して恒久的に解消する。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CheckoutSessionTest::testJsonColumnsRoundTrip が MySQL CI で失敗していた。 MySQL のネイティブ JSON 型は格納時にオブジェクトのキー順序を保持しない (PostgreSQL/SQLite は保持) ため、多キーの buyer_data に対する assertSame (順序厳密) が順序差で落ちていた。buyer_data はキーで参照する map で順序に 意味はないため、assertEqualsCanonicalizing で順序非依存に検証する。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
UCP (Universal Commerce Protocol v2026-04-08) の checkout capability を EC-CUBE 本体へ実装する。Phase 1b 共通基盤 (CheckoutSession/AgentCheckoutPurchaseFlowAdapter) の上に積み、通常購入と同一の shopping flow を再利用する。 - UcpCheckoutController: 5 エンドポイント (create/get/update/complete/cancel)。 プロトコル系=HTTP 4xx/5xx + Error、ビジネス系=HTTP 200 + messages[] の 2 系統エラー。 session 所有チェック・ucp_checkout_enabled ゲート (無効時 404)。 - インバウンド認証 = RFC 9421 HTTP Message Signatures (api4 非依存)。 UcpRequestSignatureVerifier (ES256/EC P-256・Content-Digest・ドメイン許可リスト)、 UcpProfileFetcher (HTTPS・3xx 追従禁止で signing_keys[] 取得)、署名サブスクライバ。 - マッピング: UcpCheckoutSessionMapper (totals 符号制約・代引 fee・links・暫定見積)、 UcpStatusMapper (ready↔ready_for_complete)、UcpMessageMapper (severity)。 - core 共通: AgentCheckoutPaymentHandlerRegistry + UcpPaymentHandlerInterface (具象は決済プラグインが tag で寄与)、AgentCheckoutIdempotencyStore (replay/409)。 - AgentCheckoutPurchaseFlowAdapter.complete を明示トランザクションで囲む (StockReduceProcessor の悲観ロック対応)、kana 未指定を空文字へ正規化。 検証: AgentCommerce 135 tests / 0 失敗 (incomplete 6 は意図的)、PurchaseFlow/OrderHelper 265 tests 回帰 green、PHPStan level 6 No errors、php-cs-fixer 0 件。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAgent Commerce 向けに、永続化モデル、共通サービス、UCP 署名検証と OAuth2 認可、REST API、管理設定、保護設定、テストが追加されています。 ChangesAgent Commerce UCP checkout
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
profile URL は UCP-Agent ヘッダ由来 (エージェント供給) のため、内部アドレスへ 到達可能な SSRF となり得る。多層防御として UcpProfileFetcher で以下を実施する。 - ホスト解決後の全 IP を検査し、loopback/RFC1918/link-local/reserved (FILTER_FLAG_NO_PRIV_RANGE | NO_RES_RANGE) を HTTP リクエスト前に拒否。 クラウドメタデータ (169.254.169.254) も link-local として遮断。 - 上流 HTTP ステータスコードを例外メッセージに含めない (内部探索オラクル回避)。 - DNS リゾルバを注入可能にし (既定 gethostbynamel)、テストで公開/内部解決を検証。 残存する DNS rebinding リスクは UcpRequestSignatureVerifier のドメイン許可リスト 併用で軽減する旨を docblock に明記。SSRF 遮断ケースの単体テストを追加。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…/agentic-commerce-ucp # Conflicts: # composer.lock
CI の rector (dry-run) 差分を解消する。挙動は不変。 - ResponseStatusCodeRector: HTTP ステータス数値を Response::HTTP_* 定数へ - InlineClassRoutePrefixRector: クラスレベル Route プレフィックスをメソッドへ展開 - first-class callable 化 (array_map(strtolower(...))、gethostbynamel(...) 等) - ReadOnlyClassRector: テストスタブ InMemoryUcpPaymentHandler を named readonly class へ (匿名 readonly は使わず PHP 8.2 互換を維持) - テストの assertNull→assertNotInstanceOf / 'GET'→Request::METHOD_* 等 検証: rector --dry-run クリーン、PHPStan level 6 No errors、php-cs-fixer 0 件、 AgentCommerce 141 tests 0 失敗。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (3)
src/Eccube/Service/CartService.php (1)
99-120: ⚡ Quick win
agent_owned除外条件の定義を1箇所に寄せてくださいLine 103/119 で条件を追加できている一方、同クラスの Line 389 は
cart_keyのみで再取得しています。条件が散在すると将来の修正漏れが起きやすいので、検索条件を共通化（または Line 389 にもagent_owned => falseを付与）しておくのが安全です。🔧 例（最小修正）
- $this->carts = $this->cartRepository->findBy(['cart_key' => $cartKeys], ['id' => 'ASC']); + $this->carts = $this->cartRepository->findBy(['cart_key' => $cartKeys, 'agent_owned' => false], ['id' => 'ASC']);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Eccube/Service/CartService.php` around lines 99 - 120, The `agent_owned` exclusion condition has been added to the `getPersistedCarts()` and `getSessionCarts()` methods, but the same exclusion is missing from another cart query in the same CartService class. Find all other locations in CartService where carts are retrieved from the repository (particularly any findBy() call using cart_key or similar) and add the `'agent_owned' => false` condition to ensure consistency across all cart queries. This prevents future bugs from scattered and inconsistent filtering logic.tests/Eccube/Tests/Web/AgentCommerce/UcpCheckoutControllerTest.php (1)
99-183: 署名検証失敗時（invalid signature）の 401 拒否テストを追加してくださいこのテスト群は成功系を主に検証していますが、RFC 9421 署名検証の失敗（署名改ざん・無効な署名）が HTTP 401 で拒否されることを固定していません。UcpSignatureSubscriber はリクエストが署名されている場合、署名を必ず検証し、失敗時に拒否します（
isSigned() → verify()の流れ）。同様に、requireSignatureが true に変更される場合に備えて、署名なしリクエストが 401 で拒否されることも検証する価値があります。現在は
requireSignature: falseで未署名リクエストを許容していますが、エンドポイント保護が実装されている以上、署名検証拒否ケースの固定は将来の構成変更や実装変更による回帰を防ぐのに有効です。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Eccube/Tests/Web/AgentCommerce/UcpCheckoutControllerTest.php` around lines 99 - 183, Add test methods to verify RFC 9421 signature validation failures. Create a test method (e.g., testInvalidSignatureReturns401) that makes a checkout session request with a tampered or invalid signature and asserts the response status is 401, following the same pattern as the existing test methods like testCreateGetCompleteHappyPath by using postJson() and createPayload(). Additionally, add a test method (e.g., testMissingSignatureReturns401) that sends a request without a signature header and verifies it returns 401, to ensure signature validation is properly enforced by the UcpSignatureSubscriber. These tests protect against regressions if requireSignature configuration or signature verification logic changes in the future.src/Eccube/Service/AgentCommerce/Ucp/Signature/UcpRequestSignatureVerifier.php (1)
149-179: 💤 Low valueSignature-Input パースのロバスト性について
現在の正規表現は基本的な形式に対応していますが、RFC 9421 の Structured Fields 仕様では、パラメータ値にエスケープされた引用符や複数のラベルが含まれる可能性があります。UCP の実際の使用パターンで問題がないか確認することを推奨します。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Eccube/Service/AgentCommerce/Ucp/Signature/UcpRequestSignatureVerifier.php` around lines 149 - 179, The parseSignatureInput method's regex patterns do not fully comply with RFC 9421 Structured Fields specification, specifically regarding escaped quotes within parameter values and handling of multiple labels. Review and update the regex patterns used to extract covered components (in the pattern matching quoted strings with `/"([^"]+)"/`) and the keyid parameter (in `/keyid="([^"]+)"/`) to properly handle escaped characters and edge cases as defined in RFC 9421, or implement a more robust parsing approach. Verify that the updated parsing handles the actual UCP usage patterns correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/config/eccube/services.yaml`:
- Around line 287-289: The UcpSignatureSubscriber service in services.yaml has
signature verification disabled by default with $requireSignature set to false,
which weakens authentication security. Change the default value of
$requireSignature to true to require signatures by default, and allow this to be
overridden via an environment variable only when necessary for specific
environments, ensuring that signature validation is enforced unless explicitly
disabled through configuration rather than defaulting to disabled.
In `@src/Eccube/Controller/AgentCommerce/UcpCheckoutController.php`:
- Around line 272-277: The idempotency verification in the UcpCheckoutController
does not include the agent or authenticated principal identifier in the hash
calculation, which creates a cross-border replay vulnerability where different
agents could reuse the same Idempotency-Key and receive cached responses
intended for another agent. To fix this, add the authenticated principal
information (such as ucp_agent_profile or similar agent identifier) to the array
passed to the hashRequest method call alongside the existing path, method, and
body fields. This will namespace the idempotency key by principal and prevent
response sharing across different agents.
- Around line 150-157: The order of payment capture and order confirmation is
incorrect: redeemPayment() executes before purchaseFlowAdapter->complete(),
meaning the payment can be charged even if the purchase flow completion fails
afterward, leaving billing and order status inconsistent. Restructure the code
in UcpCheckoutController at the affected locations (around lines 150-157 and
259-263) to either: (1) call redeemPayment() only after
purchaseFlowAdapter->complete() succeeds, or (2) add mandatory compensation
logic (void/refund) in the catch block when complete() or subsequent operations
fail after payment has been captured. This ensures that charges are only
finalized when the entire order process completes successfully.
In
`@src/Eccube/Service/AgentCommerce/Idempotency/AgentCheckoutIdempotencyStore.php`:
- Around line 63-79: The code has a race condition where concurrent requests
with the same Idempotency-Key can both miss the cache check and execute the
compute() function simultaneously, causing duplicate side effects. Add key-level
locking (such as a mutex or semaphore) around the critical section starting from
the cache lookup through the save operation in the method containing the
normalizeKey(), isHit(), compute(), and cache->save() calls. Ensure that once
one request acquires the lock and executes compute(), any concurrent request
with the same key will either find the cached result or wait for the lock holder
to complete and save, preventing duplicate executions.
- Line 78: The cache save operation in the AgentCheckoutIdempotencyStore class
is not handling failures properly, which breaks idempotency guarantees. When
save() is called on line 78, you must check whether the save operation succeeded
or failed. If the save operation fails, throw an exception to explicitly signal
the failure to the caller instead of silently continuing. This ensures that if
the cache persistence fails, the caller is aware and can handle the error
appropriately, preventing re-execution of side effects on retransmission.
In
`@src/Eccube/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerRegistry.php`:
- Around line 57-63: The resolveUcpByHandlerId method currently returns the
first matching handler when a handler_id is found, which creates
non-deterministic behavior when duplicate handler_ids exist across multiple
handlers (plugin conflicts). To fix this, modify the method to detect duplicates
explicitly: iterate through all handlers to count how many matches exist for the
given handlerId, then only return the handler if exactly one match is found. If
zero or multiple matches are found, throw an exception or handle the error case
explicitly to prevent the wrong handler from being silently selected. Use the
condition $handler->getHandlerId() === $handlerId to identify matches.
In `@src/Eccube/Service/AgentCommerce/Security/FilesystemKeyStore.php`:
- Around line 85-94: The resolvePath() method concatenates the $purpose
parameter directly into a file path without validation, creating a path
traversal vulnerability where malicious input like ../../../etc/passwd could
access files outside the intended directory. Add validation to restrict $purpose
to allowed characters (such as alphanumeric, underscores, and hyphens) before it
is used in the file path concatenation. You can use a whitelist approach with a
regular expression or by validating against a predefined set of allowed purpose
values, and throw an exception if the input fails validation.
In `@src/Eccube/Service/AgentCommerce/Ucp/Signature/UcpAgentHeader.php`:
- Around line 45-47: The URL scheme comparison in the parse_url and validation
logic is case-sensitive, but RFC 3986 defines URL schemes as case-insensitive.
When parse_url() extracts the scheme from a URL like HTTPS://example.com, it
returns the scheme as-is (HTTPS), causing the strict comparison with the
lowercase string 'https' to fail. Convert the scheme to lowercase using
strtolower() before comparing it against 'https' in the !== validation check to
ensure URLs with uppercase or mixed-case schemes are properly validated.
In `@src/Eccube/Service/AgentCommerce/Ucp/Signature/UcpProfileFetcher.php`:
- Around line 72-76: The httpClient->request() call in the UcpProfileFetcher
class does not include explicit timeout settings, which could cause worker
processes to hang indefinitely if the upstream server is slow. Add both timeout
and max_duration options to the options array passed to the request method to
ensure early failure when requests take too long. Set appropriate values for
both timeout (idle time) and max_duration (total execution time) to prevent
worker process stalls.
In `@src/Eccube/Service/AgentCommerce/Ucp/Signature/UcpSignatureSubscriber.php`:
- Around line 67-73: In the UcpSignatureSubscriber class where the
UcpSignatureException is caught and the reject method is called, replace the
direct use of $e->getMessage() with a generic error message to prevent leaking
internal details to the client. Instead, log the actual exception message and
details separately for debugging purposes in the server logs.
In `@src/Eccube/Service/AgentCommerce/Ucp/UcpCheckoutSessionMapper.php`:
- Around line 364-380: The method `firstDestinationPostalAddress()` returns the
entire destination object instead of extracting the postal address specifically,
causing the `extractAddress()` method to fail when the destination has a nested
`postal_address` structure. Modify the method to check for the `postal_address`
property within the first destination object and return it when present, falling
back to returning the destination object itself only if `postal_address` does
not exist, ensuring that fields like `postal_code` and `address_region` are
accessible to the calling `extractAddress()` method.
In `@tests/Eccube/Tests/Service/AgentCommerce/BaseInfoAgentCommerceFlagsTest.php`:
- Around line 62-68: The readBooleanFlag method in line 67 uses a (bool) cast on
the return value from the getter, which converts null to false and masks invalid
return types. To strictly validate the false default contract, replace the
(bool) cast with a type check that ensures the getter returns an actual boolean
value rather than null or other types. If the getter returns a non-boolean
value, the validation should fail explicitly by throwing an exception or
assertion error, rather than silently converting it to false.
In
`@tests/Eccube/Tests/Service/AgentCommerce/Idempotency/AgentCheckoutIdempotencyStoreTest.php`:
- Around line 83-88: The test method testHashRequestIsStableAcrossKeyOrder is
currently comparing the hash of an identical array twice, which does not
actually test stability across different key orders. To properly verify that the
hash function produces consistent results regardless of key order, modify the
second hashRequest call to pass an array with the same key-value pairs but with
the keys in a different order (for example, reverse the key order to ['b' => 2,
'a' => 1] while keeping the first call as ['a' => 1, 'b' => 2]). This will
correctly validate that the hashRequest method is stable across varying key
orderings.
In
`@tests/Eccube/Tests/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerRegistryTest.php`:
- Around line 43-49: The test testUcpHandlersFiltersOnlyUcp does not verify the
filtering logic properly since it only registers UCP handlers without testing
that non-UCP handlers are excluded. Modify the test to include both a UCP
handler and a non-UCP payment handler implementation in the
AgentCheckoutPaymentHandlerRegistry, then assert that ucpHandlers() returns only
the UCP handler, thereby validating that the filter correctly excludes non-UCP
implementations.
---
Nitpick comments:
In
`@src/Eccube/Service/AgentCommerce/Ucp/Signature/UcpRequestSignatureVerifier.php`:
- Around line 149-179: The parseSignatureInput method's regex patterns do not
fully comply with RFC 9421 Structured Fields specification, specifically
regarding escaped quotes within parameter values and handling of multiple
labels. Review and update the regex patterns used to extract covered components
(in the pattern matching quoted strings with `/"([^"]+)"/`) and the keyid
parameter (in `/keyid="([^"]+)"/`) to properly handle escaped characters and
edge cases as defined in RFC 9421, or implement a more robust parsing approach.
Verify that the updated parsing handles the actual UCP usage patterns correctly.
In `@src/Eccube/Service/CartService.php`:
- Around line 99-120: The `agent_owned` exclusion condition has been added to
the `getPersistedCarts()` and `getSessionCarts()` methods, but the same
exclusion is missing from another cart query in the same CartService class. Find
all other locations in CartService where carts are retrieved from the repository
(particularly any findBy() call using cart_key or similar) and add the
`'agent_owned' => false` condition to ensure consistency across all cart
queries. This prevents future bugs from scattered and inconsistent filtering
logic.
In `@tests/Eccube/Tests/Web/AgentCommerce/UcpCheckoutControllerTest.php`:
- Around line 99-183: Add test methods to verify RFC 9421 signature validation
failures. Create a test method (e.g., testInvalidSignatureReturns401) that makes
a checkout session request with a tampered or invalid signature and asserts the
response status is 401, following the same pattern as the existing test methods
like testCreateGetCompleteHappyPath by using postJson() and createPayload().
Additionally, add a test method (e.g., testMissingSignatureReturns401) that
sends a request without a signature header and verifies it returns 401, to
ensure signature validation is properly enforced by the UcpSignatureSubscriber.
These tests protect against regressions if requireSignature configuration or
signature verification logic changes in the future.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f2278db3-ed4b-468f-addd-362a8e9651fc
⛔ Files ignored due to path filters (9)
composer.lockis excluded by!**/*.locksrc/Eccube/Resource/doctrine/import_csv/en/dtb_base_info.csvis excluded by!**/*.csvsrc/Eccube/Resource/doctrine/import_csv/en/mtb_agent_protocol.csvis excluded by!**/*.csvsrc/Eccube/Resource/doctrine/import_csv/en/mtb_checkout_session_status.csvis excluded by!**/*.csvsrc/Eccube/Resource/doctrine/import_csv/en/mtb_country_iso_code.csvis excluded by!**/*.csvsrc/Eccube/Resource/doctrine/import_csv/ja/dtb_base_info.csvis excluded by!**/*.csvsrc/Eccube/Resource/doctrine/import_csv/ja/mtb_agent_protocol.csvis excluded by!**/*.csvsrc/Eccube/Resource/doctrine/import_csv/ja/mtb_checkout_session_status.csvis excluded by!**/*.csvsrc/Eccube/Resource/doctrine/import_csv/ja/mtb_country_iso_code.csvis excluded by!**/*.csv
📒 Files selected for processing (87)
.gitignore.htaccessapp/DoctrineMigrations/Version20260604120000.phpapp/DoctrineMigrations/Version20260611120000.phpapp/config/eccube/services.yamlapp/config/eccube/services_test.yamlapp/keystore/.gitkeepapp/keystore/.htaccesscomposer.jsonsrc/Eccube/Controller/AgentCommerce/UcpCheckoutController.phpsrc/Eccube/Entity/BaseInfo.phpsrc/Eccube/Entity/Cart.phpsrc/Eccube/Entity/CheckoutSession.phpsrc/Eccube/Entity/Master/AgentProtocol.phpsrc/Eccube/Entity/Master/CheckoutSessionStatus.phpsrc/Eccube/Entity/Master/CountryIsoCode.phpsrc/Eccube/Entity/Order.phpsrc/Eccube/Form/Type/Admin/ShopMasterType.phpsrc/Eccube/Repository/CheckoutSessionRepository.phpsrc/Eccube/Repository/Master/AgentProtocolRepository.phpsrc/Eccube/Repository/Master/CheckoutSessionStatusRepository.phpsrc/Eccube/Repository/Master/CountryIsoCodeRepository.phpsrc/Eccube/Resource/doctrine/import_csv/en/definition.ymlsrc/Eccube/Resource/doctrine/import_csv/ja/definition.ymlsrc/Eccube/Resource/locale/messages.en.yamlsrc/Eccube/Resource/locale/messages.ja.yamlsrc/Eccube/Resource/template/admin/Setting/Shop/shop_master.twigsrc/Eccube/Service/AgentCommerce/AddressMappingService.phpsrc/Eccube/Service/AgentCommerce/AgentCheckoutPurchaseFlowAdapter.phpsrc/Eccube/Service/AgentCommerce/CheckoutSession/AgentCheckoutAddress.phpsrc/Eccube/Service/AgentCommerce/CheckoutSession/AgentCheckoutLineItem.phpsrc/Eccube/Service/AgentCommerce/CheckoutSession/AgentCheckoutMessage.phpsrc/Eccube/Service/AgentCommerce/CheckoutSession/AgentCheckoutMessageLevel.phpsrc/Eccube/Service/AgentCommerce/CheckoutSession/AgentCheckoutRequest.phpsrc/Eccube/Service/AgentCommerce/CheckoutSession/AgentCheckoutResult.phpsrc/Eccube/Service/AgentCommerce/CheckoutSession/CustomerResolverInterface.phpsrc/Eccube/Service/AgentCommerce/CheckoutSession/GuestCustomerResolver.phpsrc/Eccube/Service/AgentCommerce/Exception/AgentCheckoutErrorCode.phpsrc/Eccube/Service/AgentCommerce/Exception/AgentCheckoutException.phpsrc/Eccube/Service/AgentCommerce/Exception/IdempotencyConflictException.phpsrc/Eccube/Service/AgentCommerce/Exception/UcpSignatureException.phpsrc/Eccube/Service/AgentCommerce/Fulfillment/FulfillmentOption.phpsrc/Eccube/Service/AgentCommerce/Fulfillment/FulfillmentOptionMapperInterface.phpsrc/Eccube/Service/AgentCommerce/Fulfillment/FulfillmentPaymentOption.phpsrc/Eccube/Service/AgentCommerce/Fulfillment/StandardFulfillmentOptionMapper.phpsrc/Eccube/Service/AgentCommerce/Idempotency/AgentCheckoutIdempotencyStore.phpsrc/Eccube/Service/AgentCommerce/MinorUnitConverter.phpsrc/Eccube/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerInterface.phpsrc/Eccube/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerRegistry.phpsrc/Eccube/Service/AgentCommerce/Payment/UcpPaymentHandlerInterface.phpsrc/Eccube/Service/AgentCommerce/Security/AgentCommerceMessageSignerInterface.phpsrc/Eccube/Service/AgentCommerce/Security/AgentCommerceOAuth2Authenticator.phpsrc/Eccube/Service/AgentCommerce/Security/AgentCommerceScopeRegistry.phpsrc/Eccube/Service/AgentCommerce/Security/FilesystemKeyStore.phpsrc/Eccube/Service/AgentCommerce/Security/KeyStoreInterface.phpsrc/Eccube/Service/AgentCommerce/Security/UcpMessageSigner.phpsrc/Eccube/Service/AgentCommerce/StorefrontUrlResolver.phpsrc/Eccube/Service/AgentCommerce/Ucp/Signature/Rfc9421SignatureBaseBuilder.phpsrc/Eccube/Service/AgentCommerce/Ucp/Signature/UcpAgentHeader.phpsrc/Eccube/Service/AgentCommerce/Ucp/Signature/UcpProfileFetcher.phpsrc/Eccube/Service/AgentCommerce/Ucp/Signature/UcpRequestSignatureVerifier.phpsrc/Eccube/Service/AgentCommerce/Ucp/Signature/UcpSignatureSubscriber.phpsrc/Eccube/Service/AgentCommerce/Ucp/UcpCheckoutSessionMapper.phpsrc/Eccube/Service/AgentCommerce/Ucp/UcpMessageMapper.phpsrc/Eccube/Service/AgentCommerce/Ucp/UcpStatusMapper.phpsrc/Eccube/Service/CartService.phptests/Eccube/Tests/Entity/CheckoutSessionTest.phptests/Eccube/Tests/Service/AgentCommerce/AddressMappingServiceTest.phptests/Eccube/Tests/Service/AgentCommerce/AgentCheckoutPurchaseFlowAdapterTest.phptests/Eccube/Tests/Service/AgentCommerce/BaseInfoAgentCommerceFlagsTest.phptests/Eccube/Tests/Service/AgentCommerce/Conformance/AgentCheckoutCoreConformanceTest.phptests/Eccube/Tests/Service/AgentCommerce/Conformance/AgentCommerceBaseConformanceTest.phptests/Eccube/Tests/Service/AgentCommerce/Conformance/UcpCheckoutConformanceTest.phptests/Eccube/Tests/Service/AgentCommerce/Fulfillment/StandardFulfillmentOptionMapperTest.phptests/Eccube/Tests/Service/AgentCommerce/Idempotency/AgentCheckoutIdempotencyStoreTest.phptests/Eccube/Tests/Service/AgentCommerce/MinorUnitConverterTest.phptests/Eccube/Tests/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerRegistryTest.phptests/Eccube/Tests/Service/AgentCommerce/Security/AgentCommerceOAuth2AuthenticatorTest.phptests/Eccube/Tests/Service/AgentCommerce/Security/AgentCommerceScopeRegistryTest.phptests/Eccube/Tests/Service/AgentCommerce/Security/UcpMessageSignerTest.phptests/Eccube/Tests/Service/AgentCommerce/Ucp/Signature/Rfc9421SignatureBaseBuilderTest.phptests/Eccube/Tests/Service/AgentCommerce/Ucp/Signature/UcpAgentHeaderTest.phptests/Eccube/Tests/Service/AgentCommerce/Ucp/Signature/UcpProfileFetcherTest.phptests/Eccube/Tests/Service/AgentCommerce/Ucp/Signature/UcpRequestSignatureVerifierTest.phptests/Eccube/Tests/Service/AgentCommerce/Ucp/UcpMessageMapperTest.phptests/Eccube/Tests/Service/AgentCommerce/Ucp/UcpStatusMapperTest.phptests/Eccube/Tests/Web/AgentCommerce/UcpCheckoutControllerTest.php
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.4 #6837 +/- ##
==========================================
+ Coverage 74.82% 75.26% +0.44%
==========================================
Files 463 497 +34
Lines 24029 25038 +1009
==========================================
+ Hits 17979 18845 +866
- Misses 6050 6193 +143
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
AgentCheckoutPurchaseFlowAdapter::complete() は validate→prepare→commit を一括実行して いたが、EC-CUBE 標準の決済フロー (在庫引当・採番=prepare → 決済オーソリ → commit/rollback, PaymentMethod が駆動) と順序が異なり、在庫検証前に決済しうる問題があった (sample-payment-plugin CreditCard の apply/checkout と乖離)。 - complete() を廃止し prepare()/commit()/rollback() を個別公開。順序とトランザクション境界の 制御は決済ハンドラ (PaymentMethod 相当) の責務とする。決済処理全体を DB トランザクションで 囲まない (EMV-3DS 等の外部サイト遷移はリクエストを跨ぐため単一トランザクションで囲えない)。 - AgentCheckoutResult に紐付け元 Cart を追加 (CheckoutSession の cart_id 設定・ pre_order_id 整合性チェック用)。 - PreOrderIdValidator::rollback はセッションレスなエージェント受注 (agent_protocol!=null) を 適用外とする。操作中カートと受注の越境防止は CheckoutSession (推測不能な session_id + protocol 照合 + cart/order の pre_order_id 整合) が controller 層で担保する。 通常購入 (agent_protocol=null) は従来どおり。ローカル検証: AgentCommerce 95 tests / PurchaseFlow 256 tests / 0 失敗、PHPStan level6 No errors、php-cs-fixer 0 件。PG/MySQL マトリクス・E2E は CI。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…entic-commerce-ucp # Conflicts: # src/Eccube/Service/AgentCommerce/AgentCheckoutPurchaseFlowAdapter.php
…tionService) 追加認証 (EMV-3DS) / escalation はエラーでなく complete が返す正常な中間状態であり、 complete は冪等な単発呼び出しでなく状態に応じて複数回呼ばれる状態機械である (EC-CUBE#6777)。 - 決済ハンドラ結果型 PaymentOutcome (COMPLETED/REQUIRES_ACTION/PENDING/FAILED) を新設し、 AgentCheckoutPaymentHandlerInterface の authorize/capture 戻り値を void から PaymentOutcome へ改訂 - AgentCheckoutCompletionService (状態機械オーケストレータ) を新設。冪等性・初回/再開フロー・ 在庫引当の保持/回収・トランザクション境界を core に集約 (prepare 成功 + authorize COMPLETED 後にのみ capture する順序を保証) - 正規化ステータスマスタに requires_action(6)/in_progress(7) を追加 (CSV + 既存環境向け INSERT migration)。 findExpired は終端 3 値のみ除外のため両者を自動的に回収対象に含む - 在庫確保期限を eccube.yaml の eccube_agent_checkout_escalation_expire (既定 15 分・EMV-3DS の 10 分より大) で設定 - AgentCheckoutPaymentHandlerRegistry (core 中立・resolveForOrder) を新設 検証: AgentCommerce+CheckoutSession 104 tests 0 失敗 (incomplete 3 意図的) / PHPStan level6 src No errors / php-cs-fixer 0 / PurchaseFlow・OrderHelper 回帰 green Refs EC-CUBE#6777 EC-CUBE#6825 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…entic-commerce-ucp # Conflicts: # app/config/eccube/services.yaml # src/Eccube/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerRegistry.php
…ure-before-commit を解消 リファクタで分割された adapter->complete() を呼び続けていた UcpCheckoutController::complete を、 Part A の AgentCheckoutCompletionService への委譲に置き換える。 - 決済 capture が注文確定前に実行され課金と注文状態が不整合になる問題 (CodeRabbit 🔴) を、 prepare→authorize→(COMPLETED 後のみ capture)→commit の順序を保証する状態機械で構造的に解消 - redeemPayment の authorize/capture 直叩きを撤去し、handler_id ベースの exchange のみ controller に残す (与信/売上は CompletionService が PaymentOutcome に従い実行) - 追加認証/escalation は requires_action 中間状態として扱い、UcpStatusMapper に requires_action→requires_escalation / in_progress→complete_in_progress を追加 - requires_action 時は continue_url (絶対 HTTPS・MUST) を付与 (ハンドラ提供分を優先・無ければ セッション取得 URL を https 強制で生成) 検証: UcpCheckoutControllerTest 6 件 green / AgentCommerce 全 151 tests 0 失敗 (incomplete 6 意図的) / PHPStan level6 No errors / php-cs-fixer 0 Refs EC-CUBE#6574 EC-CUBE#6777 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CodeRabbit レビュー指摘のうち UCP 署名系の明確な修正: - UcpAgentHeader: profile URL の scheme 比較を小文字化 (RFC 3986 で scheme は大文字小文字非区別。 HTTPS:// を誤って拒否しないように) - UcpProfileFetcher: エージェント profile 取得に timeout=5/max_duration=10 を明示し、 無応答 profile で署名検証がハングしないようにする - UcpSignatureSubscriber: 署名検証失敗時に内部エラー詳細をレスポンスへ露出させず汎用メッセージを返す (鍵/profile 取得失敗理由の情報漏洩を防ぐ) 検証: AgentCommerce 142 tests 0 失敗 / PHPStan level6 No errors / php-cs-fixer 0 Refs EC-CUBE#6574 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CodeRabbit レビュー指摘 (EC-CUBE#6574) への対応: - Idempotency: 認証済みエージェント (UCP-Agent profile) を主体としてキーを名前空間化し、 別エージェントによる同一 Idempotency-Key の越境リプレイ・キャッシュ応答共有を防ぐ - Idempotency: 同一キーの並行リクエストで副作用 (注文生成・決済) が二重実行されないよう、 キー単位の排他ロック (Symfony Lock) で hit 判定→compute→save を直列化する - hashRequest をキー順非依存 (ksort) にし、内容が同じなら同一ハッシュとする - テスト改善: Idempotency キー順序テストを逆順比較に、BaseInfo フラグ検証を bool 厳密化 (null 取りこぼし防止)、Registry の非 UCP 混在フィルタ検証と handler_id 重複時の例外を追加 検証: AgentCommerce 152 tests 0 失敗 (incomplete 6 意図的) / PHPStan level6 No errors / php-cs-fixer 0 Refs EC-CUBE#6574 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/Eccube/Service/AgentCommerce/Idempotency/AgentCheckoutIdempotencyStore.php`:
- Around line 40-44: The lock TTL is hardcoded to 30 seconds somewhere in the
AgentCheckoutIdempotencyStore class, which causes lock exclusivity to be lost
before long-running checkout processes complete, allowing other processes to
acquire the same lock key and duplicate side effects. Replace the hardcoded
30-second lock TTL value with the configurable $ttl parameter from the
constructor (which defaults to 86400 seconds) when creating or acquiring locks
via the $lockFactory. This ensures the lock TTL matches and protects the entire
cache TTL duration of the idempotency store operation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fd414d94-68db-4565-9d62-6c70c7bf22d7
⛔ Files ignored due to path filters (2)
src/Eccube/Resource/doctrine/import_csv/en/mtb_checkout_session_status.csvis excluded by!**/*.csvsrc/Eccube/Resource/doctrine/import_csv/ja/mtb_checkout_session_status.csvis excluded by!**/*.csv
📒 Files selected for processing (26)
app/DoctrineMigrations/Version20260617120000.phpapp/config/eccube/packages/eccube.yamlapp/config/eccube/services.yamlsrc/Eccube/Controller/AgentCommerce/UcpCheckoutController.phpsrc/Eccube/Entity/Master/CheckoutSessionStatus.phpsrc/Eccube/Service/AgentCommerce/AgentCheckoutPurchaseFlowAdapter.phpsrc/Eccube/Service/AgentCommerce/CheckoutSession/AgentCheckoutCompletionResult.phpsrc/Eccube/Service/AgentCommerce/CheckoutSession/AgentCheckoutCompletionService.phpsrc/Eccube/Service/AgentCommerce/CheckoutSession/AgentCheckoutResult.phpsrc/Eccube/Service/AgentCommerce/Idempotency/AgentCheckoutIdempotencyStore.phpsrc/Eccube/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerInterface.phpsrc/Eccube/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerRegistry.phpsrc/Eccube/Service/AgentCommerce/Payment/PaymentOutcome.phpsrc/Eccube/Service/AgentCommerce/Payment/PaymentOutcomeStatus.phpsrc/Eccube/Service/AgentCommerce/Ucp/Signature/UcpAgentHeader.phpsrc/Eccube/Service/AgentCommerce/Ucp/Signature/UcpProfileFetcher.phpsrc/Eccube/Service/AgentCommerce/Ucp/Signature/UcpSignatureSubscriber.phpsrc/Eccube/Service/AgentCommerce/Ucp/UcpStatusMapper.phpsrc/Eccube/Service/PurchaseFlow/Processor/PreOrderIdValidator.phptests/Eccube/Tests/Service/AgentCommerce/AgentCheckoutPurchaseFlowAdapterTest.phptests/Eccube/Tests/Service/AgentCommerce/BaseInfoAgentCommerceFlagsTest.phptests/Eccube/Tests/Service/AgentCommerce/CheckoutSession/AgentCheckoutCompletionServiceTest.phptests/Eccube/Tests/Service/AgentCommerce/Conformance/AgentCheckoutCoreConformanceTest.phptests/Eccube/Tests/Service/AgentCommerce/Conformance/UcpCheckoutConformanceTest.phptests/Eccube/Tests/Service/AgentCommerce/Idempotency/AgentCheckoutIdempotencyStoreTest.phptests/Eccube/Tests/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerRegistryTest.php
✅ Files skipped from review due to trivial changes (1)
- src/Eccube/Service/AgentCommerce/Payment/PaymentOutcomeStatus.php
🚧 Files skipped from review as they are similar to previous changes (9)
- src/Eccube/Service/AgentCommerce/CheckoutSession/AgentCheckoutResult.php
- tests/Eccube/Tests/Service/AgentCommerce/BaseInfoAgentCommerceFlagsTest.php
- src/Eccube/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerRegistry.php
- src/Eccube/Service/AgentCommerce/Ucp/Signature/UcpSignatureSubscriber.php
- src/Eccube/Entity/Master/CheckoutSessionStatus.php
- tests/Eccube/Tests/Service/AgentCommerce/Conformance/UcpCheckoutConformanceTest.php
- app/config/eccube/services.yaml
- tests/Eccube/Tests/Service/AgentCommerce/AgentCheckoutPurchaseFlowAdapterTest.php
- src/Eccube/Service/AgentCommerce/Ucp/Signature/UcpProfileFetcher.php
Idempotency 基盤をプロトコル非依存のコア (EC-CUBE#6777) として実装する。従来の cache+Symfony Lock 方式は ストアがノードローカル (SemaphoreStore/FlockStore・filesystem cache) でマルチインスタンス (AWS 等) では 越境・並行の二重実行を防げないため、DB の一意制約方式へ作り替える。 - Entity `AgentCheckoutIdempotency` (`dtb_agent_checkout_idempotency`・unique(idempotency_key, subject)) + Repository。テーブルは空始動のため schema:update 方式 (migration ファイル不要) - `AgentCheckoutIdempotencyStore` を DB ベースに実装: 予約 INSERT を一意制約で直列化し、 並行時は処理中=409 / 完了済=リプレイ / 異パラメータ=409 を判定。compute 失敗時は予約を削除し再試行可能に。 Symfony Lock は撤去 (ロック TTL 問題も解消)。subject (認証エージェント) で名前空間化し越境リプレイ防止 - `IdempotencyConflictException` をコアへ配置 (UCP/ACP 双方が利用) - 単一の共有 DB だけで動作し、Redis 等の共有キャッシュ・分散ロックに非依存 (共有レンサバ〜AWS 全形態で正しい) 検証: AgentCommerce 110 tests 0 失敗 (incomplete 3 意図的) / PHPStan level6 No errors / php-cs-fixer 0 Refs EC-CUBE#6777 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…entic-commerce-ucp # Conflicts: # src/Eccube/Service/AgentCommerce/Exception/IdempotencyConflictException.php # src/Eccube/Service/AgentCommerce/Idempotency/AgentCheckoutIdempotencyStore.php # tests/Eccube/Tests/Service/AgentCommerce/Idempotency/AgentCheckoutIdempotencyStoreTest.php
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Eccube/Service/AgentCommerce/Idempotency/AgentCheckoutIdempotencyStore.php (1)
104-110:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winネストした連想配列でハッシュが順序依存になります。
トップレベルの
ksortだけだと、ネスト階層のキー順が異なる同値ペイロードで別ハッシュになり、誤って競合扱いになります。再帰的に正規化してください。修正案
public function hashRequest(array $payload): string { - // キーの並び順に依存しない安定したハッシュにする (同一内容・異キー順を同一リクエストと扱う)。 - ksort($payload); - - return hash('sha256', (string) json_encode($payload, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)); + return hash( + 'sha256', + (string) json_encode($this->normalizePayload($payload), JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE), + ); } + + /** + * `@param` array<string|int, mixed> $payload + * + * `@return` array<string|int, mixed> + */ + private function normalizePayload(array $payload): array + { + foreach ($payload as $k => $v) { + if (is_array($v)) { + $payload[$k] = $this->normalizePayload($v); + } + } + ksort($payload); + + return $payload; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Eccube/Service/AgentCommerce/Idempotency/AgentCheckoutIdempotencyStore.php` around lines 104 - 110, The `hashRequest` method only applies `ksort` to the top-level payload array, but nested associative arrays retain their original key order, causing identical payloads with different nested key orders to produce different hashes. To fix this, implement a recursive normalization function that recursively sorts all array keys at every nesting level before hashing, ensuring that the entire payload structure is canonicalized regardless of how deeply nested the arrays are.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@src/Eccube/Service/AgentCommerce/Idempotency/AgentCheckoutIdempotencyStore.php`:
- Around line 104-110: The `hashRequest` method only applies `ksort` to the
top-level payload array, but nested associative arrays retain their original key
order, causing identical payloads with different nested key orders to produce
different hashes. To fix this, implement a recursive normalization function that
recursively sorts all array keys at every nesting level before hashing, ensuring
that the entire payload structure is canonicalized regardless of how deeply
nested the arrays are.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2b05ced8-995d-4337-b17c-3ef1f7ba6ae1
📒 Files selected for processing (8)
app/config/eccube/services.yamlapp/config/eccube/services_test.yamlsrc/Eccube/Entity/AgentCheckoutIdempotency.phpsrc/Eccube/Repository/AgentCheckoutIdempotencyRepository.phpsrc/Eccube/Service/AgentCommerce/Exception/IdempotencyConflictException.phpsrc/Eccube/Service/AgentCommerce/Idempotency/AgentCheckoutIdempotencyStore.phptests/Eccube/Tests/Service/AgentCommerce/Conformance/UcpCheckoutConformanceTest.phptests/Eccube/Tests/Service/AgentCommerce/Idempotency/AgentCheckoutIdempotencyStoreTest.php
💤 Files with no reviewable changes (1)
- app/config/eccube/services_test.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Eccube/Service/AgentCommerce/Exception/IdempotencyConflictException.php
- tests/Eccube/Tests/Service/AgentCommerce/Conformance/UcpCheckoutConformanceTest.php
CI rector が指摘した nullable instanceof アサート化・self→$this 呼び出し・ readonly プロパティ化を適用 (PreferPHPUnitThisCall / AssertEmptyNullableObjectToAssertInstanceof / AddInstanceofAssertForNullableInstance / ReadOnlyProperty)。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
15 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要(Overview・Refs Issue)
AI エージェント経由購入の UCP (Universal Commerce Protocol, v2026-04-08) checkout を EC-CUBE 本体へ実装します。Refs #6574 / 共通基盤 #6777。
Phase 1b 共通基盤 (
CheckoutSession/AgentCheckoutPurchaseFlowAdapter、#6825) の上に積むスタック PR です。#6825 着地後に4.4へ rebase 予定で、それまで本 PR の diff には #6825 の変更が含まれます。UcpCheckoutController: 5 エンドポイント (POST /ucp/checkout-sessions/GET/PUT/POST .../complete/POST .../cancel)UcpPaymentHandlerInterface+ tag レジストリのみ)方針(Policy)
specifications/ucp, v2026-04-08) を正とし、checkout-rest.md / checkout.md / signatures.md / JSON Schema に準拠。eccube.purchase.flow.shoppingを再利用。税・送料・代引手数料・在庫引当を共有 (ACP/UCP 共通基盤: エージェントコマース向け CheckoutSession・マッピング層の整備 #6777 の確定方針)。Error、ビジネス系 (在庫切れ等) = HTTP 200 +messages[]。ready ↔ ready_for_completeを変換 (マスタ非拡張)。AgentCommerceOAuth2Authenticatorは ACP /ucp:identity(api4#189) 用に温存。dtb_agent_checkout_idempotencyのunique(idempotency_key, subject)) で管理 (本 PR は consume のみ)。Symfony Lock + cache 方式から作り替え、単一の共有 DB だけでマルチインスタンス (AWS 等) の越境・並行二重実行を防止 (Redis 等の共有キャッシュ・分散ロックに非依存)。subjectに認証済みエージェントを保持し越境リプレイを防ぐ。実装に関する補足(Appendix)
AddressMappingService:getPrefFromRegion()(region→Pref 逆引き) を追加。住所マッピングを一元化。AgentCheckoutPurchaseFlowAdapter::complete():StockReduceProcessorの悲観ロックに対応するため明示トランザクションで囲む (通常購入ShoppingControllerと同方式)。UCP は kana を持たないため未指定 kana を空文字へ正規化。app/keystore/(共通の鍵保管ディレクトリapp/keystore/の導入 (各種暗号鍵・シークレットの標準置き場) #6797)。公開鍵 JWK は実行時導出。symfony/http-client(エージェント profile 取得) のみ。markTestIncompleteで明示):requires_escalation+continue_url生成、確認メール送信トレース、HTTPS 強制 (トランスポート層)、GraphQL/会員 ID 連携 (api4#189)、実 PSP 決済 (決済プラグイン側)。テスト（Test)
AgentCommerce スイート 135 tests / 765 assertions / 0 失敗 / incomplete 6 (意図的)。
UcpCheckoutConformanceTest): MUST を 1:1 トレース (status 語彙・messages severity・Idempotency replay/409)、未実装はmarkTestIncompleteUcpCheckoutControllerTest): create→get→complete ハッピーパス、住所未確定 incomplete、404 (他セッション遮断)、Idempotency-Key リプレイ、cancel、フラグ無効 404UcpRequestSignatureVerifierTest): 実 EC 鍵で RFC 9421 署名ラウンドトリップ、改竄署名・別鍵・Content-Digest 不一致・許可外ドメインを reject検証 (直接的根拠):
No errorsPostgreSQL/MySQL マトリクスは CI で確認 (ローカルは SQLite)。
相談（Discussion）
マイナーバージョン互換性保持のための制限事項チェックリスト
レビュワー確認項目
🤖 Generated with Claude Code
Summary by CodeRabbit
リリースノート