Skip to content

Commit 92b8948

Browse files
alimuzzamanclaude
andcommitted
Fix subsite enumeration bypass and PHP 5.6 compat; document PHP range in CLAUDE.md
- get_sub_sites(): use is_multisite() instead of $this->networkactive for the capability guard — a per-site-activated plugin must never let a regular subsite admin enumerate all network sites regardless of activation mode - update_options(): remove bool scalar type hint to restore PHP 5.6 compat - CLAUDE.md: add PHP Compatibility section covering features to avoid on 5.6 and deprecated patterns that cause warnings on PHP 7/8; add rule of thumb for is_multisite() vs $this->networkactive Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent f0983a4 commit 92b8948

2 files changed

Lines changed: 56 additions & 4 deletions

File tree

CLAUDE.md

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,60 @@ composer install # Install PHP dev deps (Brain/Monkey for tests)
8787

8888
---
8989

90+
## PHP Compatibility
91+
92+
**Target range: PHP 5.6 – 8.x.** Code must run without fatal errors, warnings, or deprecation notices across the full range. Two kinds of problems to avoid:
93+
94+
### Must not break on PHP 5.6 (do not use these newer features)
95+
96+
| Feature | Introduced |
97+
| ------- | ---------- |
98+
| Scalar type hints (`bool $x`, `int $x`, `: string`, `: void`) | PHP 7.0 |
99+
| Null coalescing `??` and `??=` | PHP 7.0 / 7.4 |
100+
| Spaceship operator `<=>` | PHP 7.0 |
101+
| Anonymous classes `new class` | PHP 7.0 |
102+
| `declare(strict_types=1)` | PHP 7.0 |
103+
| Nullable types `?string` | PHP 7.1 |
104+
| Array destructuring `[$a, $b] = $arr` — use `list()` | PHP 7.1 |
105+
| Typed class properties `public int $x` | PHP 7.4 |
106+
| Arrow functions `fn(` | PHP 7.4 |
107+
| Named arguments `func(name: val)` | PHP 8.0 |
108+
| Match expression `match (` | PHP 8.0 |
109+
| Nullsafe operator `?->` | PHP 8.0 |
110+
| Union types `int\|string` | PHP 8.0 |
111+
| `str_contains()`, `str_starts_with()`, `str_ends_with()` | PHP 8.0 |
112+
| Enum declarations | PHP 8.1 |
113+
| Intersection types | PHP 8.1 |
114+
| Readonly properties | PHP 8.1 |
115+
| `never` return type | PHP 8.1 |
116+
117+
Use `isset($x) ? $x : $default` instead of `$x ?? $default`.
118+
119+
### Must not trigger warnings/deprecations on PHP 7.x or 8.x (avoid these old patterns)
120+
121+
| Pattern | Removed / Deprecated |
122+
| ------- | -------------------- |
123+
| `mysql_*` functions | Removed PHP 7.0 |
124+
| `ereg()`, `split()`, `eregi()` | Removed PHP 7.0 |
125+
| Call-time pass-by-reference `foo(&$bar)` | Removed PHP 7.0 |
126+
| `/e` modifier in `preg_replace` | Removed PHP 7.0 |
127+
| `each()` | Removed PHP 8.0 |
128+
| Passing `null` to non-nullable built-in parameters | Deprecated PHP 8.1 |
129+
| Dynamic properties on non-`stdClass` objects without `#[AllowDynamicProperties]` | Deprecated PHP 8.2 |
130+
| Calling `count()` on non-countable value | Warning PHP 7.2+ |
131+
132+
Short array syntax `[]` is fine — it was introduced in PHP 5.4.
133+
134+
---
135+
90136
## Architecture Notes
91137

92138
- **Singleton pattern:** Always access via `Disable_Comments::get_instance()`.
93139
- **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`).
94140
- **Multisite vs single-site:** Plugin behaviour branches heavily on `$this->networkactive` (set in constructor) and `$this->sitewide_settings`.
95141
- **Database queries:** Use `$wpdb->prepare()` throughout `delete_comments()`. Safe against SQL injection.
96142
- **Input sanitization:** `get_form_array_escaped()` uses `wp_parse_args()` + `map_deep(sanitize_text_field)`.
143+
- **`is_multisite()` vs `$this->networkactive` — know the difference:**
144+
- `$this->networkactive` = plugin is activated network-wide. Use this for **routing** decisions: which options table to read/write, which admin menu to register, whether to show network-wide UI.
145+
- `is_multisite()` = WordPress is a multisite install (regardless of plugin activation mode). Use this for **capability guards** on any operation that touches network-level data (e.g. enumerating all sites). A per-site-activated plugin on multisite must never allow a regular subsite admin to list or access all network sites.
146+
- Rule of thumb: if the question is "where do I save this?" use `$this->networkactive`. If the question is "can this user touch network data?" use `is_multisite()`.

disable-comments.php

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ public function check_upgrades() {
273273
}
274274
}
275275

276-
private function update_options(bool $is_network_ctx = false) {
276+
private function update_options($is_network_ctx = false) {
277277
if ($this->networkactive && $is_network_ctx) {
278278
update_site_option('disable_comments_options', $this->options);
279279
} else {
@@ -1199,9 +1199,11 @@ public function get_sub_sites() {
11991199
if (!wp_verify_nonce($nonce, 'disable_comments_save_settings')) {
12001200
wp_send_json(['data' => [], 'totalNumber' => 0]);
12011201
}
1202-
// Listing subsites is a network-admin operation; require manage_network_plugins
1203-
// when the plugin is network-active, otherwise fall back to manage_options.
1204-
$required_cap = $this->networkactive ? 'manage_network_plugins' : 'manage_options';
1202+
// Listing subsites is always a network-level operation on multisite —
1203+
// require manage_network_plugins regardless of how the plugin is activated
1204+
// (network-wide or per-site). A per-site admin must never enumerate all
1205+
// network sites. On single-site installs manage_options suffices.
1206+
$required_cap = is_multisite() ? 'manage_network_plugins' : 'manage_options';
12051207
if (!current_user_can($required_cap)) {
12061208
wp_send_json(['data' => [], 'totalNumber' => 0]);
12071209
}

0 commit comments

Comments
 (0)