Enhance security checks and improve I18N handling

[alimuzzaman]

• Review all outstanding PR comments (skipping test-related ones per @alimuzzaman request)
• Fix disable_comments_settings() nonce failure silently returning success — nonce check is now an early-exit guard (matching delete_comments_settings() pattern), returning HTTP 403 instead of falling through to wp_send_json_success()
• get_sub_sites() — already restricted to manage_network_plugins on multisite with HTTP 403
• discussion_notice() — already uses wp_kses_post() to sanitize translated HTML
• disable_comments_settings() permission failure — already returns HTTP 403
• delete_comments_settings() nonce + permission failures — already return HTTP 403 with early exit
• Site Health sprintf — already uses esc_html__()
• JS networkAjaxUrl — already safely appends query param using indexOf('?') check
• is_network_admin() superglobal access — already uses $_REQUEST + wp_unslash() + sanitize_text_field()
• Code review completed — pattern is consistent with delete_comments_settings()
• PHP syntax validated

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[Copilot]

Pull request overview

This PR tightens security around multisite/network actions in the Disable Comments plugin and improves I18N + output escaping to reduce XSS risk, while adding AI/developer documentation and updating release ignore rules.

Changes:

• Added capability checks to AJAX handlers for settings save, comment deletion, and sub-site listing; tightened multisite deletion to sites a user belongs to.
• Improved escaping of role labels and dynamic UI strings in PHP/JS; adjusted a few strings for proper translation handling.
• Added developer/agent docs (CLAUDE.md, .ai/README.md) and updated .distignore / .gitignore to exclude agent artifacts from releases.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
disable-comments.php	Network permission checks, output escaping/I18N tweaks, multisite deletion restrictions, and network-admin detection change
assets/js/disable-comments-settings-scripts.js	Escapes role labels before injecting into HTML strings
CLAUDE.md	Adds plugin architecture/dev guide for contributors/agents
.gitignore	Ignores Claude worktrees and .ai/security notes
.distignore	Excludes AI/agent files and adds pnpm lockfile to dist ignore
.ai/README.md	Documents purpose/structure of the .ai/ context directory

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

get_sub_sites() currently allows either manage_network_plugins OR manage_options. In multisite setups where site admins can access the plugin settings page (e.g., when site-wide settings are enabled), this would let non-network admins enumerate all site IDs and blog names via AJAX. Consider restricting this endpoint to network admins/capability (manage_network_plugins) when is_multisite() (or otherwise gating it so it only returns data for authorized network-level users), and return a proper JSON error/HTTP status for unauthorized requests instead of an empty dataset.

Suggested change

	wp_send_json(['data' => [], 'totalNumber' => 0]);
	}
	if (!current_user_can('manage_network_plugins') && !current_user_can('manage_options')) {
	wp_send_json(['data' => [], 'totalNumber' => 0]);
	wp_send_json_error(
	[
	'message' => __('Invalid request. Please refresh the page and try again.', 'disable-comments'),
	'data' => [],
	'totalNumber' => 0,
	],
	400
	);
	}
	// Restrict access more strictly on multisite: only network admins may enumerate subsites.
	if (is_multisite()) {
	if (!current_user_can('manage_network_plugins')) {
	wp_send_json_error(
	[
	'message' => __('Sorry, you are not allowed to access this resource.', 'disable-comments'),
	'data' => [],
	'totalNumber' => 0,
	],
	403
	);
	}
	} else {
	if (!current_user_can('manage_options')) {
	wp_send_json_error(
	[
	'message' => __('Sorry, you are not allowed to access this resource.', 'disable-comments'),
	'data' => [],
	'totalNumber' => 0,
	],
	403
	);
	}

[Copilot]

New capability gating was added to the AJAX settings handlers, but there are no unit/integration tests covering the “network action requires manage_network_plugins vs. site action requires manage_options” behavior. Given this is security-sensitive logic, consider adding PHPUnit coverage to ensure unauthorized users receive an error and authorized users can still save settings.

[Copilot]

is_network_admin() previously treated multisite AJAX requests originating from Network Admin as “network” by checking the request referer, which ensured network-initiated AJAX calls loaded/saved site options correctly. After simplifying to return is_network_admin();, network admin AJAX requests routed through admin-ajax.php will likely report false, which can cause the constructor to load per-site options during network actions (e.g., affecting $old_options usage when saving network settings). Consider restoring an AJAX-safe network-context detection (e.g., checking for network admin AJAX endpoint or a verified server-side flag) or explicitly loading site options inside the network action handlers when is_network_action is true.

Suggested change

	return is_network_admin();
	// First, rely on WordPress' native network admin detection.
	if (is_network_admin()) {
	return true;
	}
	// For AJAX requests routed through admin-ajax.php, is_network_admin()
	// will often return false even when the request originated from the
	// Network Admin. In that case, fall back to checking the referer.
	if (defined('DOING_AJAX') && DOING_AJAX) {
	$referer = wp_get_referer();
	if ($referer && false !== strpos($referer, network_admin_url())) {
	return true;
	}
	}
	return false;

[Copilot]

discussion_notice() now uses __() and echoes the translated string directly while allowing embedded HTML (<em>). Since this output isn’t passed through wp_kses_post() (or otherwise escaped), a compromised/hostile translation could inject unwanted markup. Consider keeping the translation string plain text (move emphasis tags outside the translatable string) and escape with esc_html__(), or wrap the final formatted string in wp_kses_post() while ensuring dynamic pieces remain escaped.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (1)

assets/js/disable-comments-settings-scripts.js:321

• disable_comments_save_settings can now return wp_send_json_error(...) for permission failures, which will yield response.success === false in this success handler when the server responds 200. Right now the UI never resets the button / shows an error in that case, leaving it stuck in the “Saving…” state. Add handling for !response.success (and/or a .fail handler if you switch to 403 status codes) to restore the button and display the error message.

		jQuery.ajax({
			url: networkAjaxUrl,
			type: "post",
			data: data,
			beforeSend: function () {
				var btnText = __("Saving Settings..", "disable-comments");
				saveBtn.html(
					'<svg id="eael-spinner" xmlns="http://www.w3.org/2000/svg" width="20" height="20" viewBox="0 0 48 48"><circle cx="24" cy="4" r="4" fill="#fff"/><circle cx="12.19" cy="7.86" r="3.7" fill="#fffbf2"/><circle cx="5.02" cy="17.68" r="3.4" fill="#fef7e4"/><circle cx="5.02" cy="30.32" r="3.1" fill="#fef3d7"/><circle cx="12.19" cy="40.14" r="2.8" fill="#feefc9"/><circle cx="24" cy="44" r="2.5" fill="#feebbc"/><circle cx="35.81" cy="40.14" r="2.2" fill="#fde7af"/><circle cx="42.98" cy="30.32" r="1.9" fill="#fde3a1"/><circle cx="42.98" cy="17.68" r="1.6" fill="#fddf94"/><circle cx="35.81" cy="7.86" r="1.3" fill="#fcdb86"/></svg><span>' + btnText + '</span>'
				);
			},
			success: function (response) {
				if (response.success) {
					saveBtn.html(__("Save Settings", "disable-comments"));
					Swal.fire({
						icon: "success",
						title: response.data.message,
						timer: 3000,
						showConfirmButton: false,
					});
					saveBtn.removeClass('form-dirty').prop('disabled', true);
					savedData = $form.serialize();
				}
			},

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

wp_send_json_error() here doesn’t set an HTTP status code. Since this error is an authorization failure, returning a 403 status (and ensuring the JS handles non-2xx responses appropriately) makes the behavior consistent with get_sub_sites() and prevents UI states getting stuck on an error response.

Suggested change

	wp_send_json_error(['message' => __('Insufficient permissions.', 'disable-comments')]);
	wp_send_json_error(['message' => __('Insufficient permissions.', 'disable-comments')], 403);

[Copilot]

Same as above: this permission failure returns JSON without an HTTP 403 status. Using a 403 (and handling it client-side) improves correctness and aligns with the other AJAX handler (get_sub_sites) which already returns a 403 on insufficient permissions.

Suggested change

	wp_send_json_error(['message' => __('Insufficient permissions.', 'disable-comments')]);
	wp_send_json_error(['message' => __('Insufficient permissions.', 'disable-comments')], 403);

[Copilot]

This uses __() inside sprintf() for a value intended to be plain text in Site Health. Using an escaping i18n helper (e.g., esc_html__()) prevents translations from injecting markup into the debug output.

Suggested change

	'value' => sprintf(__('%1$d of %2$d', 'disable-comments'), 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']),

[Copilot]

networkAjaxUrl is built by string-concatenating '?is_network_admin=1'. If ajaxurl ever already contains a query string, this will generate an invalid URL. Consider using a small helper to append the parameter with ?/& correctly (or build via URL/URLSearchParams).

Suggested change

	var networkAjaxUrl = ajaxurl + (disableCommentsObj.is_network_admin === '1' ? '?is_network_admin=1' : '');
	var appendQueryParam = function (url, key, value) {
	var separator = url.indexOf('?') === -1 ? '?' : '&';
	return url + separator + encodeURIComponent(key) + '=' + encodeURIComponent(value);
	};
	var networkAjaxUrl = disableCommentsObj.is_network_admin === '1'
	? appendQueryParam(ajaxurl, 'is_network_admin', '1')
	: ajaxurl;

[Copilot]

is_network_admin() reads $_GET['is_network_admin'] directly in an AJAX context. In WP admin requests, superglobals may be slashed and should be accessed via wp_unslash() + sanitization (or strict validation) to avoid PHPCS violations and inconsistent behavior. Consider reading from $_REQUEST, unslashing/sanitizing, and then doing the '1' comparison.

Suggested change

	return !empty($_GET['is_network_admin'])
	&& $_GET['is_network_admin'] === '1'
	$is_network_admin = '';
	if (isset($_REQUEST['is_network_admin'])) {
	$is_network_admin = wp_unslash($_REQUEST['is_network_admin']);
	if (!is_string($is_network_admin)) {
	$is_network_admin = '';
	}
	$is_network_admin = sanitize_text_field($is_network_admin);
	}
	return ('1' === $is_network_admin)

[Copilot]

This notice now outputs a raw translated string containing HTML via __() and echoes it without any escaping/sanitization. To avoid translator-introduced XSS while still allowing intended markup (e.g., <em>), sanitize the final HTML with wp_kses_post() (or wp_kses() with an explicit allowlist) and keep dynamic pieces escaped as they already are.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

disable-comments.php:1183

• get_sub_sites() computes $required_cap for non-multisite installs, but it still calls get_sites() unconditionally. Since the handler is registered on all installs, calling this AJAX action on a single-site setup can trigger a fatal error if get_sites() isn’t available. Add an early is_multisite() / function_exists('get_sites') guard and return a JSON error when multisite APIs aren’t present.

		$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 = [];
		$type = isset($_GET['type']) ? sanitize_text_field(wp_unslash($_GET['type'])) : 'disabled';
		$search = isset($_GET['search']) ? sanitize_text_field(wp_unslash($_GET['search'])) : '';
		$pageSize = isset($_GET['pageSize']) ? sanitize_text_field(wp_unslash($_GET['pageSize'])) : 50;
		$pageNumber = isset($_GET['pageNumber']) ? sanitize_text_field(wp_unslash($_GET['pageNumber'])) : 1;
		$offset = ($pageNumber - 1) * $pageSize;
		$sub_sites = get_sites([
			'number' => $pageSize,
			'offset' => $offset,
			'search' => $search,

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

delete_comments_settings() also returns wp_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.

[Copilot]

The new permission logic and multisite membership filtering in delete_comments_settings() isn’t covered by tests. Add PHPUnit tests to assert 403s for insufficient capability and nonce failure, plus a multisite test where a non-super-admin cannot delete comments on a site they’re not a member of (ensuring those sites are skipped/blocked as intended).

[alimuzzaman]

@copilot review again. But skip tests for now.

[Copilot]

@copilot review again. But skip tests for now.

Reviewed all actionable non-test comments. Most were already addressed in prior commits. The one remaining issue found: disable_comments_settings() had the same nonce-failure-silently-returns-success bug that was fixed in delete_comments_settings() — a failed nonce would skip the inner if block but still hit wp_send_json_success(). Fixed in 8693705 by restructuring to use the same early-exit pattern: nonce is now verified before the main logic block, sending HTTP 403 and dying immediately on failure.