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:

```php
'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`

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