Hi-School-Slide-Manager/AUDIT-REPORT.md

Hi-School Slide Manager — Plugin Audit Report

Date: February 10, 2025
Context: WordPress site using Divi 5 theme
Plugin version: 1.1.1

---

Executive summary

The plugin is well-structured with clear separation (CPT, REST API, Admin, Frontend, Shortcode, Slider). Security basics are in place (nonces, capability checks, sanitization). Several issues should be fixed: a missing REST controller class, unsafe REST API image handling, unprofessional file in the package, production error_log usage, a broken API key delete UI, and incomplete uninstall. Divi 5 compatibility is likely fine but should be confirmed in your environment.

---

1. Critical issues

1.1 Missing REST controller class (CPT)

File: includes/class-hssm-slides-cpt.php (line 113)

The slides CPT is registered with:

'rest_controller_class' => 'HSSM_REST_Slides_Controller',

The class HSSM_REST_Slides_Controller is not defined anywhere in the plugin. WordPress may throw a fatal error or warning when the REST API is used for this post type, or fall back to default behavior unpredictably.

Recommendation: Either:

• Remove rest_controller_class and rest_base to use the default WP_REST_Posts_Controller, or
• Implement HSSM_REST_Slides_Controller extending WP_REST_Posts_Controller and register it (and ensure the class is loaded before register_post_type).

---

1.2 REST API — possible PHP notice/error in image sizes

File: includes/class-hssm-rest-api.php (lines 386–391)

In get_slide_background_image(), the sizes array is built by calling wp_get_attachment_image_src( $thumbnail_id, 'large' )[0] (and similarly for medium, thumbnail). If a size does not exist (e.g. image too small), wp_get_attachment_image_src() returns false, and [0] on false can cause a notice or error.

Recommendation: For each size, check the return value before using [0], and only add the URL when the result is valid (e.g. use a helper that returns $src ? $src[0] : '' or omit the key).

---

1.3 API key delete — wrong key can be deleted

File: includes/class-hssm-admin.php (api_keys_field)

Each API key row outputs:

• A “Delete” submit button (same name="delete_api_key" value="1" for all rows).
• A hidden input name="api_key_index" value="<?php echo $index; ?>".

With multiple keys, there are multiple api_key_index inputs. On submit, PHP receives a single $_POST['api_key_index']; with multiple fields with the same name, the value is often the last one in the form. So clicking “Delete” on the first key can submit the index of the last key and delete the wrong key.

Recommendation: Use one hidden input for api_key_index and set its value via JavaScript when the user clicks a specific row’s Delete button. Alternatively, use a separate form per row or a single delete endpoint that takes the key index as a parameter (with nonce and capability checks).

---

2. Security and best practices

2.1 Good practices observed

• Direct access: All PHP files guard with ABSPATH check.
• Nonces: Admin settings and AJAX use nonces (hssm_settings, hssm_dashboard_nonce, hssm_update_slide_order).
• Capabilities: Settings require manage_options; slide actions use edit_posts, delete_post, edit_post, upload_files, etc., as appropriate.
• Sanitization: Inputs sanitized with sanitize_text_field, esc_url_raw, wp_kses_post, absint, etc.
• Escaping in output: Templates and admin use esc_html, esc_attr, esc_url, esc_js where needed.
• REST API: Protected endpoints use a custom permission callback and API key validation; parameters are validated and sanitized.

2.2 Public AJAX test endpoint

File: includes/class-hssm-frontend.php

add_action( 'wp_ajax_nopriv_hssm_test', array( $this, 'ajax_test' ) );

ajax_test() only returns a success message, but it is callable by unauthenticated users. It is unnecessary in production and could be used to probe for the plugin.

Recommendation: Remove the wp_ajax_nopriv_hssm_test registration (or restrict it to WP_DEBUG / admin-only) before production.

2.3 AJAX handlers and $_POST / $_FILES

Some handlers assume $_POST['nonce'] or other keys exist (e.g. ajax_upload_image, ajax_save_slide, ajax_preview_slides). If a request is sent without the nonce, wp_verify_nonce( $_POST['nonce'], ... ) can trigger an “undefined index” notice.

Recommendation: Use isset( $_POST['nonce'] ) (and similar for other keys) before using them, and return a clear error response when required parameters are missing.

2.4 Settings form and options.php

The settings form uses action="" and does not use options.php. Saving is handled in handle_settings_save() on POST. That’s valid, but ensure all saved options are registered with register_setting and that no extra options are written without sanitization (currently looks consistent).

---

3. Uninstall and cleanup

File: includes/class-hssm-main.php — remove_plugin_options()

Removed options are:

• hssm_notification_emails, hssm_default_slide_duration, hssm_cache_duration, hssm_api_version, hssm_preview_page_slug, hssm_manager_page_slug, hssm_api_keys

Not removed:

• hssm_plugin_mode
• hssm_api_url
• hssm_client_site_slug
• hssm_notification_settings (used by HSSM_Notifications::get_settings())

Recommendation: Add these option names to the list in remove_plugin_options() so uninstall leaves no HSSM options behind (and document that uninstall removes all plugin data).

---

4. Code quality and maintainability

4.1 Production error_log usage

File: includes/class-hssm-frontend.php (and possibly others)

Multiple error_log() calls are used for debugging (e.g. “HSSM: AJAX handlers registered”, “Raw form data received”, “Validation failed”, “Schedule slide AJAX called”). These run for every request/hit and can clutter logs and leak internal details.

Recommendation: Remove or wrap all error_log() calls in if ( defined( 'WP_DEBUG' ) && WP_DEBUG && defined( 'WP_DEBUG_LOG' ) && WP_DEBUG_LOG ) (or a small helper) so they only run when debug logging is enabled.

4.2 Duplicate / redundant comment

File: includes/class-hssm-frontend.php

There are two consecutive docblocks for “AJAX handler for image uploads” (around ajax_upload_image and before sanitize_slide_data). One is redundant and can be removed for clarity.

4.3 REST API date filter vs. status

File: includes/class-hssm-rest-api.php — get_slides_for_site()

When $status === 'scheduled' or 'all', the existing meta_query built for $date can be overwritten. If both date and status are passed, the date filter may not apply in those branches. Worth a quick logic review so date and status filters compose as intended.

---

5. Files and packaging

5.1 Unprofessional file in plugin directory

File: fuck-you-google.html (root of plugin)

This file exists in the plugin directory (content is empty). The name is unprofessional and should not be distributed or deployed.

Recommendation: Delete this file from the plugin and from any repo or deployment package.

5.2 Development / internal docs

The plugin contains internal docs such as FRONTEND_SETUP.md, mop-up.md, Notifications.md, prompt-builder-reference-and-notes.md, and mimic-carousel.html. These are fine for development but could be excluded from production builds or moved to a /docs folder so the plugin root stays clean.

---

6. Divi 5 compatibility

• The plugin uses standard WordPress mechanisms: shortcode [hssm_slider], page templates (with fallback to plugin templates), get_header() in templates, and normal enqueue/localize for CSS/JS. None of this is Divi-specific.
• Divi 5 continues to support shortcodes and standard theme hooks, so the plugin is expected to work with Divi 5.
• Recommendation: Manually test on a Divi 5 site:
  • Place [hssm_slider] in a Divi layout (e.g. Code module or shortcode module).
  • Use the “Slide Dashboard” and “Slide Preview” page templates and confirm layout and scripts (e.g. datepicker, slider) behave correctly.
  • If Divi 5 uses different wrapper classes or JS namespaces, minor CSS/JS tweaks might be needed; no such issues were evident from code review alone.

---

7. Summary checklist

Area	Status	Notes
Security (nonces, caps, sanitize/escape)	OK	A few improvements recommended (nopriv test, isset checks).
REST API	Fix	Missing CPT REST controller; image sizes may error.
Admin settings	Fix	API key delete can delete wrong key.
Uninstall	Fix	Add missing options to cleanup.
Logging	Fix	Restrict or remove error_log in production.
Packaging	Fix	Remove fuck-you-google.html.
Divi 5	Verify	Expected to work; confirm with testing.

---

8. Recommended fix order

1. Immediate: Remove fuck-you-google.html from the plugin directory.
2. High: Fix or remove rest_controller_class for the slides CPT (implement or drop custom REST controller).
3. High: Harden get_slide_background_image() so missing image sizes never cause notices/errors.
4. High: Fix API key delete so the correct key is removed (single hidden + JS or per-row form).
5. Medium: Remove or restrict wp_ajax_nopriv_hssm_test and add isset() checks for AJAX parameters.
6. Medium: Add all HSSM options to uninstall cleanup; wrap or remove error_log() for production.
7. Low: Clean duplicate docblock and optional doc/file organization.

If you want, I can suggest concrete code changes (diffs) for any of these items next.