Skip to content

Fix Session Fixation: ログアウト時のセッションID再生成を追加#1343

Merged
nanasess merged 3 commits intomasterfrom
fix/security-session-fixation
Feb 20, 2026
Merged

Fix Session Fixation: ログアウト時のセッションID再生成を追加#1343
nanasess merged 3 commits intomasterfrom
fix/security-session-fixation

Conversation

@nobuhiko
Copy link
Contributor

@nobuhiko nobuhiko commented Feb 12, 2026

Summary

  • SC_Customer::EndSession() にてセッション変数のクリア後に SC_Session_Ex::regenerateSID() を呼び出し、セッションIDを再生成するように修正
  • ログイン時には既に regenerateSID() が呼ばれているが、ログアウト時には欠如していた

Test plan

  • PHPUnit全テスト通過(MySQL & PostgreSQL)
  • E2Eテスト全テスト通過(MySQL & PostgreSQL)
  • ログイン→ログアウト→再ログインが正常動作すること

Refs #1336

🤖 Generated with Claude Code

Summary by CodeRabbit

  • セキュリティ改善
    • ログアウト時に新しいセッションIDが発行されるようになりました。

Add SC_Session_Ex::regenerateSID() call in EndSession() to invalidate the
old session ID when a customer logs out, preventing session fixation attacks.

Refs #1336

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 12, 2026

No actionable comments were generated in the recent review. 🎉


📝 Walkthrough

Walkthrough

SC_Customer.phpのEndSessionメソッドに、セッションデータ破棄後かつログ記録前にセッションID再生成を呼び出すコードが追加されました。セッション固定化攻撃を防止するため、ログアウト時に新しいセッションIDを発行する変更です。

Changes

Cohort / File(s) Summary
Session Security Enhancement
data/class/SC_Customer.php
EndSessionメソッドにセッションID再生成処理を追加。セッション破棄後に新しいSIDを発行することでセッション固定化を防止。

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 セッションの終わり、新たに始まり
古き ID は忘れ去られて
固定化の鬼から身を守る
再生成の魔法で安心と共に
兎も応援する、セキュリティの輝き✨

🚥 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/class/SC_Customer.php (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 プルリクエストのタイトルは、セッション固定化(Session Fixation)の脆弱性に対する具体的な修正内容を明確に表現しており、ログアウト時のセッションID再生成の追加という主要な変更を適切に要約しています。
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/security-session-fixation
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch fix/security-session-fixation
  • 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

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Feb 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.42%. Comparing base (f5e0dc0) to head (726fd7f).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1343      +/-   ##
==========================================
+ Coverage   54.40%   54.42%   +0.02%     
==========================================
  Files          84       84              
  Lines       10816    10817       +1     
==========================================
+ Hits         5884     5887       +3     
+ Misses       4932     4930       -2     
Flag Coverage Δ
tests 54.42% <100.00%> (+0.02%) ⬆️

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
Copy link
Contributor

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Feb 16, 2026

✅ 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.

Copy link
Contributor

@nanasess nanasess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nanasess nanasess enabled auto-merge February 20, 2026 02:37
@nanasess nanasess merged commit 0295256 into master Feb 20, 2026
334 of 335 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments