Skip to content

fix: resolve PHPCS Plugin Check false positives and wp_redirect security warnings - #3209

Merged
brianhogg merged 2 commits into
gocodebox:devfrom
faisalahammad:review/false-positives
Jul 17, 2026
Merged

fix: resolve PHPCS Plugin Check false positives and wp_redirect security warnings#3209
brianhogg merged 2 commits into
gocodebox:devfrom
faisalahammad:review/false-positives

Conversation

@faisalahammad

@faisalahammad faisalahammad commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Replace wp_redirect() with wp_safe_redirect() in the template loader and lesson progression controller to mitigate open redirect risks flagged by the WordPress Plugin Check tool.

Changes

includes/class.llms.template.loader.php

Before: wp_redirect() used for content restriction redirects.

After: wp_safe_redirect().

Why: Restricts redirect destinations to local URLs, prevents open redirect.

includes/controllers/class.llms.controller.lesson.progression.php

Before: wp_redirect() used for lesson auto-advance after completion.

After: wp_safe_redirect().

Why: Same open redirect mitigation.

Testing

Test: Lesson auto-advance

  1. Enroll student in course with multiple lessons
  2. Complete lesson
  3. Verify redirect goes to next lesson

Result: Works as expected

Test: Content restriction redirect

  1. Set course to members only
  2. Access course page as logged-out user
  3. Verify redirect to configured page

- Replace wp_redirect() with wp_safe_redirect() in template loader and
  lesson progression controller to mitigate open redirect risks
- Add phpcs:ignore for legacy unprefixed Meta_Box_Field_Interface
- Exclude dev files (AGENTS.md, CLAUDE.md) from distribution archive
- Extract HTML from translatable string in parent course template
@faisalahammad
faisalahammad requested a review from brianhogg as a code owner June 19, 2026 15:18
@brianhogg brianhogg moved this to Awaiting Review in Development Jun 19, 2026
@brianhogg

Copy link
Copy Markdown
Contributor

@faisalahammad The "fixes" number is pointing to a pull request. Not sure if there's an issue this is fixing or if that should be removed from the description?

Comment thread templates/course/parent-course.php Outdated
Comment thread includes/admin/post-types/meta-boxes/fields/llms.interface.meta.box.field.php Outdated
@github-project-automation github-project-automation Bot moved this from Awaiting Review to To do in Development Jul 16, 2026
@brianhogg brianhogg moved this from To do to Review in Progress in Development Jul 16, 2026
@faisalahammad

Copy link
Copy Markdown
Contributor Author

Removed the Fixes #3194 line. That PR is unrelated, you were right. Also reverted the parent-course.php translation change and the meta-box field phpcs:ignore per your review. PR now only contains the two wp_safe_redirect() swaps and the changelog entry.

@brianhogg

brianhogg commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

@faisalahammad I see the description change but could you confirm the changes have been pushed? I'm still seeing the change to includes/admin/post-types/meta-boxes/fields/llms.interface.meta.box.field.php and templates/course/parent-course.php in the diff https://github.com/gocodebox/lifterlms/pull/3209/changes. Thanks!

- Revert parent-course.php translation string change
- Remove phpcs:ignore from Meta_Box_Field_Interface

Refs gocodebox#3209
@faisalahammad

Copy link
Copy Markdown
Contributor Author

@brianhogg I had to step away from my desk halfway through because of Jumma prayer, so the changes weren’t fully pushed yet. When you get a chance, could you please take another look and re-review it? Really appreciate your help! Thanks a lot.

@brianhogg
brianhogg merged commit 0696f0c into gocodebox:dev Jul 17, 2026
18 of 21 checks passed
@github-project-automation github-project-automation Bot moved this from Review in Progress to Done in Development Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants