Fix XSS: 管理画面の商品選択テンプレートでJSコンテキストのエスケープを追加

[nobuhiko]

Summary

• product_select.tpl の JavaScript 文字列内で tpl_no, shipping_id に |escape:'javascript' を適用
• onclick ハンドラ内の tpl_class_name1, tpl_class_name2 に |escape:'javascript' を適用
• HTMLコンテキスト内のエラーメッセージで規格名に |h フィルタを追加

※ file_manager.tpl の tpl_now_dir は PHP 側で jsonEncode() 済みのため安全
※ design/index.tpl の page_id は |h + NUM_CHECK 済みのため安全

Test plan

• PHPUnit全テスト通過（MySQL & PostgreSQL）
• E2Eテスト全テスト通過（MySQL & PostgreSQL）
• 管理画面の受注編集→商品選択ポップアップが正常動作すること

Refs #1335

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes

• 管理画面の商品選択フォームにおいて、フォーム値の割り当てと検索結果表示の処理を改善しました。

---

語数: 23語

[coderabbitai]

📝 Walkthrough

ウォークスルー

Smarty テンプレートファイルにおいて、JavaScript コンテキスト内で使用される変数に対して |escape:'javascript' フィルタを適用し、JavaScript インジェクション対策を実装しました。フォームフィールドの値割り当てと onclick ハンドラーの引数渡しが更新されています。

変更内容

コホート / ファイル	概要
JavaScript コンテキストにおける変数のエスケープ data/Smarty/templates/admin/order/product_select.tpl	tpl_no、shipping_id、tpl_class_name1、tpl_class_name2 に対して JavaScript エスケープフィルタを適用し、フォーム値の割り当てと onclick ハンドラーの引数をセキュアに変更。HTML エスケープも適切に適用。

推定コードレビュー工数

🎯 2 (Simple) | ⏱️ ~10分

ポエム

🐰 ピクピク

セキュリティの盾を張り巡らせ
JavaScript の危険から守り抜く
エスケープの魔法で、ホップ・ステップ・ジャンプ！
テンプレートは今、より安全に
キャロットのように甘く、しっかりと🥕✨

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Merge Conflict Detection	⚠️ Warning	❌ Merge conflicts detected (6 files): ⚔️ composer.json (content) ⚔️ composer.lock (content) ⚔️ data/Smarty/templates/admin/order/product_select.tpl (content) ⚔️ data/class/pages/mypage/LC_Page_Mypage_Delivery.php (content) ⚔️ data/module/HTTP/Request.php (content) ⚔️ data/module/Net/URL.php (content) These conflicts must be resolved before merging into master.	Resolve conflicts locally and push changes to this branch.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	タイトルは、JavaScriptコンテキストのXSS脆弱性を修正するために管理画面の商品選択テンプレートにエスケープ処理を追加するという、変更セットの主要な目的を明確に要約している。
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/security-xss-admin-js-context

⚔️ Resolve merge conflicts (beta)

• Auto-commit resolved conflicts to branch fix/security-xss-admin-js-context
• Create stacked PR with resolved conflicts
• Post resolved changes as copyable diffs in a comment

---

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 and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.39%. Comparing base (1c777c9) to head (5f1c78a).
⚠️ Report is 18 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1345      +/-   ##
==========================================
- Coverage   54.72%   54.39%   -0.33%     
==========================================
  Files          84       84              
  Lines       10815    10815              
==========================================
- Hits         5918     5883      -35     
- Misses       4897     4932      +35     

Flag	Coverage Δ
tests	54.39% <ø> (-0.33%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nanasess]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

🧹 Nitpick comments (1)

data/Smarty/templates/admin/order/product_select.tpl (1)

233-233: onclick 属性内の |escape:'javascript' はHTMLエンティティのデコード後に評価されるため、二重コンテキストの考慮が必要です。

onclick="..." はまずHTMLパーサーが属性値を解析し、その後JSとして評価されます。|escape:'javascript' は " を \" に変換しますが、HTMLパーサーにとって " は属性の終了と解釈される可能性があります。

理論的には |escape:'javascript'|h のチェーンで、JS → HTML の順にエスケープすべきです。ただし、規格名に " が含まれるケースは実運用上まれであり、管理画面限定のため、リスクは低いです。

♻️ より安全なエスケープチェーンの提案

-                    <td class="center"><a href="javascript:;" onclick="return func_submit('<!--{$arrProducts[cnt].product_id|h}-->', '<!--{$tpl_class_name1[$id]|escape:'javascript'}-->', '<!--{$tpl_class_name2[$id]|escape:'javascript'}-->'); return false;">決定</a></td>
+                    <td class="center"><a href="javascript:;" onclick="return func_submit('<!--{$arrProducts[cnt].product_id|h}-->', '<!--{$tpl_class_name1[$id]|escape:'javascript'|h}-->', '<!--{$tpl_class_name2[$id]|escape:'javascript'|h}-->'); return false;">決定</a></td>