Skip to content

Conversation

@iodic
Copy link
Contributor

@iodic iodic commented Oct 3, 2025

Description

This change updates the session initialization logic in FacebookWordpressOpenBridge::start_new_php_session_if_needed() to set the $secure flag dynamically based on whether the site is served over HTTPS — $secure = is_ssl().

Previously, $secure was hardcoded to false, which could cause cookies to be sent over insecure connections even on HTTPS sites.

This PR improves compliance with WordPress security best practices and prevents insecure session cookies from being set.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have commented my code, particularly in hard-to-understand areas.
  • I have confirmed that my changes do not introduce any new PHPCS warnings or errors.
  • I have checked plugin debug logs that my changes do not introduce any new PHP warnings or FATAL errors.
  • I followed general Pull Request best practices. Meta employees to follow this wiki.
  • I have added tests (if necessary) and all the new and existing unit tests pass locally with my changes.
  • I have completed dogfooding and QA testing, or I have conducted thorough due diligence to ensure that it does not break existing functionality.
  • I have updated or requested update to plugin documentations (if necessary).

Changelog entry

Fix insecure session cookies by setting $secure based on is_ssl()

Test Plan

  1. Set up a WordPress site with HTTPS enabled.
  2. Enable the plugin and trigger session creation (e.g. visit site with Pixel events firing).
  3. Inspect the browser’s developer tools under Application -> Storage -> Cookies.
  4. Confirm that the session cookie has: Secure = true when the site is served over HTTPS. Secure = false when the site is served over HTTP.
  5. Check debug logs to ensure no new warnings or errors are introduced.

@meta-cla meta-cla bot added the cla signed label Oct 3, 2025
@iodic iodic requested a review from vahidkay-meta October 3, 2025 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants