audit clean-up
add 'no dates' logic
This commit is contained in:
+190
@@ -0,0 +1,190 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user