-
Notifications
You must be signed in to change notification settings - Fork 32
Enhance security checks and improve I18N handling #160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
alimuzzaman
wants to merge
22
commits into
master
Choose a base branch
from
alim-dev
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
95e00ff
Improve I18N Issue based on 2.5.3
alexclassroom a60017c
Merge branch 'WPDevelopers:master' into master
alexclassroom e6fd88a
Merge branch 'WPDevelopers:master' into master
alexclassroom 24752f0
Merge branch 'WPDevelopers:master' into master
alexclassroom acbc92f
Add AI agent context, CLAUDE.md, and dev tooling exclusions
alimuzzaman c6714a3
Security: remove HTTP_REFERER trust from is_network_admin() #4
alimuzzaman e3ed4b9
Security: add capability check to disable_comments_settings() #1
alimuzzaman 8921f5d
Security: add capability and per-blog auth checks to delete_comments_…
alimuzzaman 854eb86
Security: escape role names to prevent DOM XSS in role exclusion UI #3
alimuzzaman 6cd801a
Security: add capability check to get_sub_sites() to prevent subsite …
alimuzzaman 60bd5f3
Exclude .claude/ worktrees from git tracking and distribution zips
alimuzzaman c9b578a
Security: derive network context from POST data, not is_network_admin()
alimuzzaman 9e9fe6d
Exclude pnpm-lock.yaml from distribution
alimuzzaman 6f54eea
Merge pull request #156 from alexclassroom/master
alimuzzaman 9fa11c9
Fix esc_html__ stripping HTML tags in admin notice
alimuzzaman b996182
Security: gate update_site_option on network action capability check
alimuzzaman 6e5a776
Fix is_network_admin() AJAX context and unify network action detection
alimuzzaman 78478d0
Address Copilot review suggestions from PR #160
alimuzzaman 2c86f7f
Fix UI error handling and double-encoding in role names
alimuzzaman 5e6657a
Handle new server error responses in JS
alimuzzaman e41335c
Fix nonce failure silently returning success in delete_comments_setti…
alimuzzaman 8693705
Fix nonce failure silently returning success in disable_comments_sett…
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| # .ai/ — Agent Context Directory | ||
|
|
||
| This directory stores documentation and context generated or maintained by AI agents (Claude Code, etc.) during development work. | ||
|
|
||
| ## Purpose | ||
|
|
||
| - **Deep-dive architecture docs** — detail too verbose for `CLAUDE.md` | ||
| - **Investigation notes** — written while exploring complex features | ||
| - **Feature context** — background and rationale for non-obvious design decisions | ||
|
|
||
| This is NOT a replacement for code comments, git history, or `docs/`. It supplements `CLAUDE.md` with detail that would otherwise bloat it. | ||
|
|
||
| > **Agents:** When you discover context worth preserving, write it here — not into `CLAUDE.md`. Reference the file from `CLAUDE.md` with a one-line pointer. | ||
|
|
||
| ## Structure | ||
|
|
||
| ```text | ||
| .ai/ | ||
| README.md # This file — directory overview | ||
| security/ # Vulnerability reports and fix guidance (excluded from git) | ||
| ``` | ||
|
|
||
| ## Rules for Agents | ||
|
|
||
| - **Write here, not in `CLAUDE.md`** — keep CLAUDE.md as a concise rules + pointers file | ||
| - **One file per topic** — don't append unrelated notes to an existing file | ||
| - **This directory is excluded from distribution zips** — `.distignore` covers `.ai/` recursively |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,5 +8,7 @@ node_modules/ | |
| vendor/ | ||
| .vscode/ | ||
| .idea | ||
| .claude/worktrees/ | ||
| package-lock.json | ||
| pnpm-lock.yaml | ||
| .ai/security/ | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| # Disable Comments — Plugin Development Guide | ||
|
|
||
| WordPress plugin by WPDeveloper. Allows administrators to globally disable comments by post type, with multisite network support. | ||
|
|
||
| - **WordPress.org:** <https://wordpress.org/plugins/disable-comments/> | ||
| - **Current version:** 2.6.2 | ||
| - **Main class:** `Disable_Comments` (singleton) in `disable-comments.php` | ||
|
|
||
| --- | ||
|
|
||
| ## Project Structure | ||
|
|
||
| ```text | ||
| disable-comments.php Main plugin file (~2000 lines), single class | ||
| includes/ | ||
| cli.php WP-CLI command definitions | ||
| class-plugin-usage-tracker.php | ||
| views/ | ||
| settings.php Main settings page shell | ||
| comments.php Tools/delete page shell | ||
| partials/ | ||
| _disable.php Disable-comments form (main settings form) | ||
| _delete.php Delete comments form | ||
| _sites.php Multisite sub-site list | ||
| _menu.php / _footer.php / _sidebar.php | ||
| assets/ | ||
| js/disable-comments-settings-scripts.js Settings page JS (role exclusion UI, AJAX calls) | ||
| js/disable-comments.js | ||
| css/ scss/ | ||
| tests/ | ||
| test-plugin.php PHPUnit tests (Brain/Monkey mocking) | ||
| bootstrap.php | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## Key AJAX Handlers | ||
|
|
||
| All three AJAX handlers are registered in `__construct()` (~line 49): | ||
|
|
||
| | Action | Handler | Line | | ||
| | ------ | ------- | ---- | | ||
| | `disable_comments_save_settings` | `disable_comments_settings()` | ~1217 | | ||
| | `disable_comments_delete_comments` | `delete_comments_settings()` | ~1324 | | ||
| | `get_sub_sites` | `get_sub_sites()` | ~1157 | | ||
|
|
||
| **Nonce:** All handlers verify nonce `disable_comments_save_settings`. The nonce is created in `admin_enqueue_scripts()` (~line 799) and exposed to JS as `disableCommentsObj._nonce`. | ||
|
|
||
| **POST data parsing:** `get_form_array_escaped()` (~line 1202) reads `$_POST['data']` as a URL-encoded string, parses with `wp_parse_args()`, and sanitizes all values with `map_deep(..., 'sanitize_text_field')`. | ||
|
|
||
| **Network admin flag:** `$formArray['is_network_admin']` comes from POST data and controls network-wide operations — always verify server-side capability before acting on it. | ||
|
|
||
| --- | ||
|
|
||
| ## Development | ||
|
|
||
| ```bash | ||
| npm install # Install JS build deps | ||
| npm run build # Compile JS/CSS via Grunt + Babel | ||
| npm run release # Build + generate .pot + package release | ||
| ``` | ||
|
|
||
| ```bash | ||
| composer install # Install PHP dev deps (Brain/Monkey for tests) | ||
| ./vendor/bin/phpunit # Run tests | ||
| ``` | ||
|
|
||
| **Linting:** `phpcs.ruleset.xml` is configured for WordPress Coding Standards. | ||
|
|
||
| --- | ||
|
|
||
| ## Architecture Notes | ||
|
|
||
| - **Singleton pattern:** Always access via `Disable_Comments::get_instance()`. | ||
| - **CLI support:** `includes/cli.php` calls the same handler methods with `$_args` to bypass nonce (expected for WP-CLI context; nonce bypass is gated on `$this->is_CLI`). | ||
| - **Multisite vs single-site:** Plugin behaviour branches heavily on `$this->networkactive` (set in constructor) and `$this->sitewide_settings`. | ||
| - **Database queries:** Use `$wpdb->prepare()` throughout `delete_comments()`. Safe against SQL injection. | ||
| - **Input sanitization:** `get_form_array_escaped()` uses `wp_parse_args()` + `map_deep(sanitize_text_field)`. |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -116,10 +116,13 @@ function __construct() { | |
| } | ||
|
|
||
| public function is_network_admin() { | ||
| $sanitized_referer = isset($_SERVER['HTTP_REFERER']) ? sanitize_text_field(wp_unslash($_SERVER['HTTP_REFERER'])) : ''; | ||
| if (is_network_admin() || !empty($sanitized_referer) && defined('DOING_AJAX') && DOING_AJAX && is_multisite() && preg_match('#^' . network_admin_url() . '#i', $sanitized_referer)) { | ||
| if (is_network_admin()) { | ||
| return true; | ||
| } | ||
| if (defined('DOING_AJAX') && DOING_AJAX) { | ||
| $is_network_admin_param = isset($_REQUEST['is_network_admin']) ? sanitize_text_field(wp_unslash($_REQUEST['is_network_admin'])) : ''; | ||
| return $is_network_admin_param === '1' && current_user_can('manage_network_plugins'); | ||
| } | ||
| return false; | ||
| } | ||
| /** | ||
|
|
@@ -796,7 +799,8 @@ public function settings_page_assets($hook_suffix) { | |
| 'save_action' => 'disable_comments_save_settings', | ||
| 'delete_action' => 'disable_comments_delete_comments', | ||
| 'settings_URI' => $this->settings_page_url(), | ||
| '_nonce' => wp_create_nonce('disable_comments_save_settings') | ||
| '_nonce' => wp_create_nonce('disable_comments_save_settings'), | ||
| 'is_network_admin' => is_network_admin() ? '1' : '0' | ||
| ) | ||
| ); | ||
| wp_set_script_translations('disable-comments-scripts', 'disable-comments'); | ||
|
|
@@ -829,7 +833,7 @@ public function discussion_notice() { | |
| } | ||
|
|
||
| // translators: %s: disabled post types. | ||
| echo '<div class="notice notice-warning"><p>' . sprintf(esc_html__('Note: The <em>Disable Comments</em> plugin is currently active, and comments are completely disabled on: %s. Many of the settings below will not be applicable for those post types.', 'disable-comments'), implode(esc_html__(', ', 'disable-comments'), $names_escaped)) . '</p></div>'; | ||
| echo '<div class="notice notice-warning"><p>' . wp_kses_post(sprintf(__('Note: The <em>Disable Comments</em> plugin is currently active, and comments are completely disabled on: %s. Many of the settings below will not be applicable for those post types.', 'disable-comments'), implode(__(', ', 'disable-comments'), $names_escaped))) . '</p></div>'; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1159,7 +1163,12 @@ public function settings_page() { | |
| public function get_sub_sites() { | ||
| $nonce = (isset($_REQUEST['nonce']) ? sanitize_text_field(wp_unslash($_REQUEST['nonce'])) : ''); | ||
| if (!wp_verify_nonce($nonce, 'disable_comments_save_settings')) { | ||
| wp_send_json(['data' => [], 'totalNumber' => 0]); | ||
| wp_send_json_error(['message' => __('Invalid request. Please refresh the page and try again.', 'disable-comments')], 403); | ||
| } | ||
|
|
||
| $required_cap = is_multisite() ? 'manage_network_plugins' : 'manage_options'; | ||
| if (!current_user_can($required_cap)) { | ||
| wp_send_json_error(['message' => __('Sorry, you are not allowed to access this resource.', 'disable-comments')], 403); | ||
| } | ||
|
|
||
| $_sub_sites = []; | ||
|
|
@@ -1219,14 +1228,21 @@ public function disable_comments_settings($_args = array()) { | |
| if (($this->is_CLI && !empty($_args)) || wp_verify_nonce($nonce, 'disable_comments_save_settings')) { | ||
|
|
||
| $formArray = $this->get_form_array_escaped($_args); | ||
| $is_network_action = $this->is_CLI | ||
| ? (!empty($formArray['is_network_admin']) && $formArray['is_network_admin'] == '1') | ||
| : $this->is_network_admin(); | ||
| $required_cap = $is_network_action ? 'manage_network_plugins' : 'manage_options'; | ||
| if (!$this->is_CLI && !current_user_can($required_cap)) { | ||
| wp_send_json_error(['message' => __('Insufficient permissions.', 'disable-comments')], 403); | ||
| } | ||
|
|
||
| $old_options = $this->options; | ||
| $old_options = $this->is_CLI ? $this->options : ($is_network_action ? get_site_option('disable_comments_options', []) : $this->options); | ||
| $this->options = []; | ||
| if ($this->is_CLI) { | ||
| $this->options = $old_options; | ||
| } | ||
|
|
||
| $this->options['is_network_admin'] = isset($formArray['is_network_admin']) && $formArray['is_network_admin'] == '1' ? true : false; | ||
| $this->options['is_network_admin'] = $is_network_action; | ||
|
|
||
| if (!empty($this->options['is_network_admin']) && function_exists('get_sites') && empty($formArray['sitewide_settings'])) { | ||
| $formArray['disabled_sites'] = isset($formArray['disabled_sites']) ? $formArray['disabled_sites'] : []; | ||
|
|
@@ -1256,12 +1272,12 @@ public function disable_comments_settings($_args = array()) { | |
| $this->options['extra_post_types'] = array_diff($extra_post_types, array_keys($post_types)); // Make sure we don't double up builtins. | ||
| } | ||
|
|
||
| if (isset($formArray['sitewide_settings'])) { | ||
| if ($is_network_action && isset($formArray['sitewide_settings'])) { | ||
| update_site_option('disable_comments_sitewide_settings', $formArray['sitewide_settings']); | ||
| } | ||
|
|
||
| if (isset($formArray['disable_avatar'])) { | ||
| if ($this->is_network_admin()) { | ||
| if ($is_network_action) { | ||
| if ($formArray['disable_avatar'] == '0' || $formArray['disable_avatar'] == '1') { | ||
| $sites = get_sites([ | ||
| 'number' => 0, | ||
|
|
@@ -1328,15 +1344,28 @@ public function delete_comments_settings($_args = array()) { | |
|
|
||
| if (($this->is_CLI && !empty($_args)) || wp_verify_nonce($nonce, 'disable_comments_save_settings')) { | ||
| $formArray = $this->get_form_array_escaped($_args); | ||
| $is_network_action = $this->is_CLI | ||
| ? (!empty($formArray['is_network_admin']) && $formArray['is_network_admin'] == '1') | ||
| : $this->is_network_admin(); | ||
|
|
||
| if (!$this->is_CLI) { | ||
| $required_cap = $is_network_action ? 'manage_network_plugins' : 'manage_options'; | ||
| if (!current_user_can($required_cap)) { | ||
| wp_send_json_error(['message' => __('Insufficient permissions.', 'disable-comments')], 403); | ||
| } | ||
| } | ||
|
|
||
| if (!empty($formArray['is_network_admin']) && function_exists('get_sites') && class_exists('WP_Site_Query')) { | ||
| if ($is_network_action && function_exists('get_sites') && class_exists('WP_Site_Query')) { | ||
| $sites = get_sites([ | ||
| 'number' => 0, | ||
| 'fields' => 'ids', | ||
| ]); | ||
| foreach ($sites as $blog_id) { | ||
| // $formArray['disabled_sites'] ids don't include "site_" prefix. | ||
| if (!empty($formArray['disabled_sites']) && !empty($formArray['disabled_sites']["site_$blog_id"])) { | ||
| if (!is_super_admin() && !is_user_member_of_blog(get_current_user_id(), $blog_id)) { | ||
| continue; // Skip sites the user doesn't belong to | ||
| } | ||
|
Comment on lines
+1361
to
+1383
|
||
| switch_to_blog($blog_id); | ||
| $log = $this->delete_comments($_args); | ||
| restore_current_blog(); | ||
|
|
@@ -1850,7 +1879,7 @@ public function add_site_health_info($debug_info) { | |
| ), | ||
| 'disabled_post_type_count' => array( | ||
| 'label' => __('Disabled Post Types Count', 'disable-comments'), | ||
| 'value' => sprintf('%d of %d', count($data['disabled_post_types']), $data['total_post_types']), | ||
| 'value' => sprintf(esc_html__('%1$d of %2$d', 'disable-comments'), count($data['disabled_post_types']), $data['total_post_types']), | ||
| ), | ||
| 'disabled_post_types' => array( | ||
| 'label' => __('Disabled Post Types', 'disable-comments'), | ||
|
|
||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete_comments_settings()also returnswp_send_json_success()even when the nonce check fails, because the success response is sent unconditionally after the guarded block. This can report a successful deletion (often with a meaningless “.” message) for invalid/expired nonces. Send a JSON error (HTTP 403) and exit when nonce verification fails.