feat/security audit remediation#2
Merged
Merged
Conversation
kevinchappell
commented
May 22, 2026
kevinchappell
commented
Collaborator
- add marketing
- docs: comprehensive security audit report (4 HIGH, 7 MEDIUM, 5 LOW)
- fix(HH-2): separate API key sanitization callbacks per provider
- fix(HH-3): enable SSL verification on cURL image fetching
- fix(HH-4): correct function name mismatches causing fatal errors
- migrate to git s
- fix(MM-1,MM-3): escape debug output + gate all error_log calls
- fix(MM-5,MM-6,LL-1): strengthen encryption + remove stale constants
- fix(MM-4,LL-4): remove unused nonce fields + gate debug section
- fix(MM-7): harden SSRF protection against DNS rebinding
- fix(LL-3): add rate limiting to AJAX endpoints
- fix(LL-2): sanitize AI-generated content before storing in block attributes
- docs: mark all security audit issues as remediated
The kwik_ai_tags_sanitize_api_key() function was storing the same input value to BOTH kwik_ai_openrouter_api_key and kwik_ai_openai_api_key, causing saving one provider's key to overwrite the other. Fix: Create three separate sanitization functions: - kwik_ai_tags_sanitize_openrouter_api_key() - kwik_ai_tags_sanitize_openai_api_key() - kwik_ai_tags_sanitize_custom_api_key() Also adds secure credential storage for the custom/Ollama API key which was previously stored in plaintext.
Remove CURLOPT_SSL_VERIFYPEER=false which disabled TLS certificate validation, allowing man-in-the-middle attacks on image data. Also adds MIME type validation (image/*) to all image fetching methods (wp_remote_get, cURL) to prevent non-image content from being sent to AI models.
Fix calls to non-existent functions: - kwik_ai_tags_query_ollama() -> kwik_ai_tags_query_ai() - kwik_ai_tags_query_ollama_text_only() -> kwik_ai_tags_query_ai_text_only() - kwik_ai_tags_query_text_only() -> kwik_ai_tags_query_ai_text_only() Affected files: web-scraping.php, description-generation.php, tag-generation.php These mismatches caused fatal 'call to undefined function' errors when URL-based description generation or text-only tag generation was triggered.
MM-1: Escape word count ternary output in meta-boxes.php by assigning to variable first, then escaping with esc_html(). MM-3: Replace all raw error_log() calls with kwik_ai_log() helper that gates on WP_DEBUG. This prevents disk exhaustion from verbose logging on production sites. Created kwik_ai_log() in core/functions.php as a centralized logging helper. Replaced 88 error_log() calls across: - includes/tag-generation.php (29) - includes/description-generation.php (33) - includes/image-processing.php (18) - admin/admin-init.php (8) - admin/meta-boxes.php (6)
MM-5: Replace deterministic wp_hash_salt() encryption key with a random key stored in wp_options (autoload=no). An attacker needs both database AND the stored key to decrypt credentials. MM-6: Replace base64 fallback (zero security) with HMAC-SHA256 one-way hashing. Primary method is OpenSSL AES-256-CBC for two-way encryption when available. LL-1: Remove KWIK_AI_AI_PROVIDER, KWIK_AI_API_ENDPOINT, KWIK_AI_API_KEY, KWIK_AI_MODEL, KWIK_AI_OPENROUTER_API_KEY, KWIK_AI_OPENAI_API_KEY constants which were frozen on first load and never reflected settings changes. Code already uses getter functions for these values.
MM-4: Remove wp_nonce_field() from meta boxes since they are
AJAX-driven and nonce verification happens in ajax-handlers.php
via check_ajax_referer(). The nonce fields suggested an incomplete
save_post handler that never existed.
LL-4: Gate debug info sections behind current_user_can('manage_options')
in addition to WP_DEBUG to prevent information disclosure to lower-
privileged users who can edit posts.
- Remove pre-fetch DNS resolution (gethostbyname) which is vulnerable to DNS rebinding attacks where the attacker returns a safe IP during validation, then a private IP during the actual fetch. - Add kwik_ai_is_private_ip() helper for reusable IP validation. - Disable HTTP redirects (redirection => 0) in wp_remote_get to prevent redirect-based SSRF where the initial URL is safe but the redirect target is internal. - Replace remaining error_log() calls with kwik_ai_log() in web-scraping.php.
Add kwik_ai_check_rate_limit() helper using WordPress transients to prevent abusive users from hammering AI API endpoints. Rate limit: 1 request per 15 seconds per user for: - Tag generation (kwik_ai_tags_generate) - Description generation (kwik_ai_description_generate) Also replaces remaining error_log() calls with kwik_ai_log() in ajax-handlers.php for consistent debug logging.
…ibutes Add sanitize_textarea_field() to strip HTML/JavaScript from AI-generated descriptions before sending them to the client via AJAX response. Also validates and sanitizes URL input with esc_url_raw() and FILTER_VALIDATE_URL to prevent injection attacks.
Update SECURITY-AUDIT.md to reflect that all 16 identified issues (4 HIGH, 7 MEDIUM, 5 LOW) have been fixed and verified with tests.
There was a problem hiding this comment.
Pull request overview
Security-audit remediation for the Kwik AI WordPress plugin, combining code hardening (SSRF/XSS/credential handling/logging/AJAX abuse) with updated defaults/configuration and new unit tests to enforce key security patterns.
Changes:
- Refactors settings/provider handling (default provider, per-provider API key sanitization, removes dynamic constants) and introduces centralized gated logging.
- Hardens request surfaces (SSRF protections in web scraping, SSL verification on cURL image fetch, MIME validation, rate limiting for AJAX, sanitization of AI-generated content).
- Adds/updates PHPUnit tests and documentation to reflect and verify remediation work.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
tests/unit/SecurityTest.php |
Adds static-analysis style unit tests to assert presence of security mitigations in source. |
tests/unit/KwikAiSettingsTest.php |
Updates expected default provider to custom. |
tests/unit/KwikAiIntegrationTest.php |
Updates constant expectations and asserts dynamic settings are not constants. |
tests/unit/KwikAiApiTest.php |
Updates provider expectations and API config tests. |
tests/bootstrap.php |
Updates mocked option defaults to align with new settings/provider behavior. |
SECURITY-AUDIT.md |
Adds a comprehensive audit report and remediation status tracking. |
phpunit.xml |
Simplifies test suites to the unit test directory. |
MARKETING.md |
Adds marketing/overview documentation. |
includes/web-scraping.php |
Refactors SSRF validation and fetch behavior for URL scraping. |
includes/tag-generation.php |
Switches logging to centralized helper and routes requests through provider-agnostic query helpers. |
includes/security-utilities.php |
Reworks credential encryption/decryption and key management. |
includes/ollama-api.php |
Shifts default provider behavior toward custom and updates auth/header logic accordingly. |
includes/image-processing.php |
Adds MIME validation and removes disabling of SSL verification for cURL fetch. |
includes/description-generation.php |
Switches logging to centralized helper and routes requests through provider-agnostic query helpers. |
core/functions.php |
Adds kwik_ai_log() helper to gate logging behind WP_DEBUG. |
core/constants.php |
Removes dynamic get_option()-based constants; keeps only static defaults. |
BRANCH-GUIDE.md |
Updates documentation to reflect auth/config changes (API key vs username/password). |
blocks/description-block.php |
Sanitizes block output to mitigate stored XSS risk. |
assets/js/settings.js |
Replaces minified script with readable code and adds provider-based API key field toggling. |
admin/settings.php |
Updates settings registration (provider defaults, API key fields) and adds enabled post type UI. |
admin/meta-boxes.php |
Gates debug output and reduces production logging. |
admin/ajax-handlers.php |
Adds per-user rate limiting and sanitizes AI output before returning/storing. |
admin/admin-init.php |
Moves debug logging to centralized gated logger. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+29
to
36
| // Sanitize the description to prevent stored XSS | ||
| // AI-generated content may contain HTML; allow safe markup only | ||
| $safe_description = wp_kses_post($description); | ||
|
|
||
| return sprintf( | ||
| '<div class="wp-block-kwik-ai-description">%s</div>', | ||
| $description | ||
| esc_html($safe_description) | ||
| ); |
Comment on lines
18
to
38
| function kwik_ai_validate_url_safety($url) { | ||
| $parsed_url = parse_url($url); | ||
|
|
||
| if (!$parsed_url || !isset($parsed_url['host'])) { | ||
| return false; | ||
| } | ||
|
|
||
| $host = $parsed_url['host']; | ||
|
|
||
| // Resolve hostname to IP | ||
| $ip = gethostbyname($host); | ||
|
|
||
| if ($ip === $host) { | ||
| // Could not resolve hostname | ||
| return false; | ||
| } | ||
|
|
||
| // Block private IP ranges (RFC 1918) | ||
| $private_ranges = [ | ||
| '/^10\./', // 10.0.0.0/8 | ||
| '/^172\.(1[6-9]|2[0-9]|3[0-1])\./', // 172.16.0.0/12 | ||
| '/^192\.168\./', // 192.168.0.0/16 | ||
| '/^127\./', // Loopback | ||
| '/^169\.254\./', // Link-local | ||
| ]; | ||
|
|
||
| foreach ($private_ranges as $pattern) { | ||
| if (preg_match($pattern, $ip)) { | ||
| error_log('Kwik AI: Blocked SSRF attempt to private IP: ' . $ip . ' (from ' . $host . ')'); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
|
|
||
| // Block IPv6 loopback and link-local | ||
| if (strpos($host, '::1') === 0 || strpos($host, 'fe80:') === 0) { | ||
| error_log('Kwik AI: Blocked SSRF attempt to IPv6 private address: ' . $host); | ||
| kwik_ai_log('Kwik AI: Blocked SSRF attempt to IPv6 private address: ' . $host); | ||
| return false; | ||
| } | ||
|
|
||
| // Additional check: ensure URL uses http/https | ||
| if (!isset($parsed_url['scheme']) || !in_array(strtolower($parsed_url['scheme']), ['http', 'https'])) { | ||
| return false; | ||
| } | ||
|
|
||
| return true; |
Comment on lines
75
to
126
| @@ -170,18 +117,61 @@ function kwik_ai_fetch_url_content(string $url): ?string | |||
|
|
|||
| $content = @file_get_contents($url, false, $context); | |||
| if ($content !== false) { | |||
| if (defined('WP_DEBUG') && WP_DEBUG) { | |||
| error_log('Kwik AI: Successfully fetched content via file_get_contents (' . strlen($content) . ' chars)'); | |||
| } | |||
| kwik_ai_log('Kwik AI: Successfully fetched content via file_get_contents (' . strlen($content) . ' chars)'); | |||
| return $content; | |||
| } | |||
|
|
|||
| if (defined('WP_DEBUG') && WP_DEBUG) { | |||
| error_log('Kwik AI: file_get_contents failed'); | |||
| } | |||
| kwik_ai_log('Kwik AI: file_get_contents failed'); | |||
| return false; | |||
| } | |||
Comment on lines
+20
to
+32
| function kwik_ai_get_encryption_key(): string | ||
| { | ||
| $key_option = 'kwik_ai_encryption_key'; | ||
| $key = get_option($key_option, ''); | ||
|
|
||
| if (empty($key)) { | ||
| // Generate a cryptographically secure random key | ||
| $key = bin2hex(random_bytes(32)); | ||
| add_option($key_option, $key, '', 'no'); | ||
| } | ||
|
|
||
| return $key; | ||
| } |
Comment on lines
+84
to
+90
| // Primary: OpenSSL AES-256-CBC decryption | ||
| if (function_exists('openssl_encrypt')) { | ||
| $data = base64_decode($value); | ||
| if ($data === false) { | ||
| return ''; | ||
| } | ||
|
|
Comment on lines
+63
to
66
| // Fallback: HMAC-based one-way storage (not reversible, but secure) | ||
| // Use hash_hmac instead of plain base64 which provides zero security | ||
| return 'hmac:' . hash_hmac('sha256', $value, $key); | ||
| } |
Comment on lines
129
to
133
| function kwik_ai_tags_get_ollama_config() | ||
| { | ||
| $url = get_option('kwik_ai_api_endpoint', 'http://localhost:11434'); | ||
| $username = get_option('kwik_ai_tags_ollama_username', ''); | ||
| $password = get_option('kwik_ai_tags_ollama_password', ''); | ||
| $api_key = get_option('kwik_ai_custom_api_key', ''); | ||
|
|
Comment on lines
+297
to
+307
| function kwik_ai_tags_sanitize_openai_api_key($input) | ||
| { | ||
| $key = trim($input); | ||
|
|
||
| // Store securely if available | ||
| if (function_exists('kwik_ai_store_credential')) { | ||
| kwik_ai_store_credential('kwik_ai_openai_api_key', $key); | ||
| } | ||
|
|
||
|
|
||
| return $key; | ||
| } |
Comment on lines
+316
to
326
| function kwik_ai_tags_sanitize_custom_api_key($input) | ||
| { | ||
| $key = trim($input); | ||
|
|
||
| // Store securely if available | ||
| if (function_exists('kwik_ai_store_credential')) { | ||
| kwik_ai_store_credential('kwik_ai_custom_api_key', $key); | ||
| } | ||
|
|
||
| return $key; | ||
| } |
Comment on lines
+75
to
+83
| function kwik_ai_decrypt(string $value): string | ||
| { | ||
| // HMAC-stored values are one-way and cannot be decrypted | ||
| if (strpos($value, 'hmac:') === 0) { | ||
| return ''; // Cannot decrypt one-way hashes | ||
| } | ||
|
|
||
| $key = kwik_ai_get_encryption_key(); | ||
|
|
- Rewrite phpcs.xml from invalid PHP to proper XML ruleset format - Focus on PHPCompatibility and critical WordPress security sniffs - Fix NonceVerification: move check_ajax_referer before debug logging - Fix EscapeOutput: use esc_html__() in wp_die() calls - Fix ValidatedSanitizedInput: add isset() checks before $_POST access - Fix MissingUnslash: use wp_unslash() before sanitization - Fix InputNotSanitized: use array_map with sanitize_text_field on URL arrays - Exclude non-PHP files (JS/CSS) and test scripts from linting - Add .gitignore for .phpcs.cache
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.