fix: upgrade scify/laravel-cookie-guard to 5.1.0 - #402
Conversation
Version 4.x stored the consent cookie for 30 days, although the configuration sets 365. It posted to /guard-settings/save outside the web middleware group, so the CSRF token was never checked. It hid the banner before the server confirmed the save. It did not erase the cookies of a rejected category. It also spells the plural interval as [2,Inf], which Laravel 12.48 and newer print literally in the banner. - Require scify/laravel-cookie-guard ^5.1.0. - Re-publish the cookies-consent-public assets. The fix is in that JavaScript. - Publish those assets on every composer update, through post-update-cmd. - Remove the enabled config key. Version 5 removed the option.
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe pull request upgrades the cookie guard package, adds automatic asset publication, removes the enabled-category override, updates consent handling, and regenerates the published CSS and JavaScript assets. ChangesCookie consent update
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant CookieConsentUI
participant scripts.js
participant ConsentServer
participant BrowserCookies
CookieConsentUI->>scripts.js: Submit selected consent categories
scripts.js->>ConsentServer: Send consent with XSRF or CSRF header
ConsentServer-->>scripts.js: Return consent data
scripts.js->>BrowserCookies: Store returned consent and remove declined cookies
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 1 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #402 +/- ##
=========================================
Coverage 50.17% 50.17%
Complexity 1394 1394
=========================================
Files 195 195
Lines 4759 4759
=========================================
Hits 2388 2388
Misses 2371 2371 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 participants
Version 4.x stored the consent cookie for 30 days, although the
configuration sets 365. It posted to /guard-settings/save outside the web
middleware group, so the CSRF token was never checked. It hid the banner
before the server confirmed the save. It did not erase the cookies of a
rejected category. It also spells the plural interval as [2,Inf], which
Laravel 12.48 and newer print literally in the banner.
JavaScript.
The consent cookie keeps its name, so visitors are not asked again. Consent
cookies written by 4.x hold 30 days and expire on their own schedule. New
consents last the configured 365 days.
Summary by CodeRabbit
New Features
Bug Fixes
Style