Found by the adversarial review of PR #54 (2.3.2 safety release). Verified — the reviewer
attempted to refute this and could not.
webdecoy.php:1484 · severity high · lens applied-fixes
The defect
Excluding THROTTLE from record_suppressed_action() makes the monitor-mode notice assert "No request has met the bar for enforcement yet" on a default install where rate limiting (enabled by default) is the only thing being suppressed — the number the owner uses to decide whether enforcement is safe omits the action most likely to hit real traffic, and the write-avoidance rationale is void because a detections INSERT already happens per throttled request.
Failure scenario
Fresh 2.3.2 install, defaults: monitor_mode=true, rate_limit_enabled=true, rate_limit_requests=60, rate_limit_window=60, no tripwire hits. An aggressive third-party crawler (or an office NAT) issues 120 front-end requests/minute from one IP. WebDecoy_Rate_Limit_Rule returns THROTTLE (includes/class-webdecoy-rate-limit-rule.php:68) for ~60 requests/minute; handle_rule_decision() suppresses the 429 and, because of line 1484, records nothing. webdecoy_suppressed_actions stays empty, so render_state_notices() prints the $total === 0 branch (webdecoy.php:514): "Everything is detected and logged; nothing is blocked. No request has met the bar for enforcement yet." The owner reads that as "turning on enforcement changes nothing", disables monitor mode, and the plugin immediately starts serving 429s to that traffic. The comment's premise that THROTTLE "previously performed no database write on this path" is false: log_rule_violations() at webdecoy.php:1066 inserts a webdecoy_detections row for every violation, THROTTLE included, before handle_rule_decision() runs — so the flood it seeks to avoid already exists, and the skipped write is a single-row option UPDATE that is cheaper than the INSERT already performed.
Verified mechanism
Every link in the chain checks out on fix/2.3.2-safety (HEAD 6f26683).
- Defaults (webdecoy.php ~230-249):
monitor_mode => true, rate_limit_enabled => true, rate_limit_requests => 60, rate_limit_window => 60, rate_limit_algorithm => 'fixed', rate_limit_key => 'ip', rate_limit_dry_run => false. So on a fresh install the rate limiter is armed and non-dry-run.
build_rule_engine() webdecoy.php:1208-1216 appends WebDecoy_Rate_Limit_Rule whenever rate_limit_enabled, so the engine is non-null even with nothing else configured.
WebDecoy_Rate_Limit_Rule::evaluate() returns $this->action = THROTTLE (constructor line 68, result lines 90-104) once check_and_increment() reports current > limit. Fixed window keyed on the bare IP (line 135), so 120 req/min from one address yields ~60 THROTTLEs/min.
RuleEngine::evaluate() (sdk/src/Rules/RuleEngine.php:69-96) records a ViolationEvent for every non-ALLOW result — THROTTLE included — and makes it the deciding result.
early_check() webdecoy.php:1066 calls log_rule_violations() for all violations, and that method does one $wpdb->insert into webdecoy_detections per violation (webdecoy.php:1608-1616). Confirmed on main too (git show main:webdecoy.php:894), so the comment's "it previously performed no database write on this path" is false. It is worse than the claim states: the default fixed-window limiter already does a SELECT plus an UPDATE/INSERT on webdecoy_rate_limits for the same request (includes/class-webdecoy-rate-limiter.php:91-107). Three DB touches already occur; the one being avoided is a fourth.
- webdecoy.php:1484 skips
record_suppressed_action() exactly as claimed.
webdecoy_suppressed_actions has exactly one reader — render_state_notices() webdecoy.php:496 — verified by grep across the whole plugin. It sums only count and prints the literal string "No request has met the bar for enforcement yet." at line 512 when the sum is 0, or understates at line 509 when it is not.
- The comment's fallback justification, "The rate limiter's own counters already carry that number", is also false.
webdecoy_rate_limits stores only the current window's counter and the row is deleted and re-inserted each new window (rate-limiter.php:102-107); it carries no history. Nothing in admin/ renders it — grep of admin/ for rate_limit returns only the settings-form inputs.
So the notice can state, on a default install, that nothing has met the bar for enforcement while ~60 requests/minute are being throttle-suppressed. The more common manifestation on a public site is the other branch: tripwire hits get counted so the total is non-zero, and the owner is shown a number that omits the throttles — which on a fast crawler or an office/CGNAT address is the dominant term by orders of magnitude. The owner reads "would have acted on 12 requests in 30 days", disables monitor mode, and the site starts serving 429s. Note also that the good-bot exemption is checked after the engine short-circuits (webdecoy.php:1094, reached only when the engine allows), so a fast Googlebot/Bingbot crawl is throttle-eligible — this is precisely the "blocked my customers/SEO" outcome the release exists to prevent, and it is the one class of suppressed action the counter cannot show.
The whole suppression mechanism (monitor_mode, suppression_reason(), record_suppressed_action(), render_state_notices()) is new in this branch — git show main:webdecoy.php | grep monitor_mode is empty — so this is a new-in-branch defect, not inherited. No test asserts the exclusion (grep of tests/ for suppressed_action returns nothing), so the fix is safe against the 75/75 suite.
Two corrections to the claim's framing, neither of which changes the verdict: (a) the changelog points owners at the Detections page, and throttle violations do land there as rows flagged rate-limit:60/60s_rule, so the data is not wholly invisible — but the notice's assertion is still false, which is the stated blocker criterion; (b) the fix is not simply deleting the if. $result->reason for the limiter embeds the varying counter ("Rate limit exceeded: 61/60 requests in 60s window", rate-limit-rule.php:94), so an unnormalized record would add a fresh key to $stats[$day]['by'] on every throttled request and run the arsort + array_slice at webdecoy.php:1449-1452 each time. The reason must be normalized to the stable rule name.
Fix
webdecoy.php:1479-1489 — count THROTTLE, but with a stable reason key so the breakdown map cannot churn. Replace the comment and the if with:
// Counted for THROTTLE too: rate limiting is the action most likely to
// meet real traffic, so omitting it makes the monitor-mode number the
// owner decides on meaningless. Keyed on the rule name, not the reason,
// because the reason embeds the request counter (a fresh key per
// request). The request already writes to webdecoy_rate_limits and
// inserts a webdecoy_detections row, so this adds no new order of cost.
$this->record_suppressed_action(
$suppression,
$result->action === \WebDecoy\Rules\RuleResult::THROTTLE
? ('Throttled: ' . ($result->rule ?? 'rate-limit'))
: ($result->reason ?? ('Rule enforced: ' . ($result->rule ?? 'rule')))
);
$result->rule here is the limiter's getName() ("rate-limit:60/60s", class-webdecoy-rate-limit-rule.php:74), which is constant for a given configuration.
Separately, while in this method: the docblock at webdecoy.php:1434 says "a single autoloaded counter", but line 1458 passes false for autoload. Reword to "a single non-autoloaded counter" so the next reader does not go looking for an autoload problem.
Blocks the 2.3.2 release. Refs #54.
Found by the adversarial review of PR #54 (2.3.2 safety release). Verified — the reviewer
attempted to refute this and could not.
webdecoy.php:1484 · severity
high· lensapplied-fixesThe defect
Excluding THROTTLE from record_suppressed_action() makes the monitor-mode notice assert "No request has met the bar for enforcement yet" on a default install where rate limiting (enabled by default) is the only thing being suppressed — the number the owner uses to decide whether enforcement is safe omits the action most likely to hit real traffic, and the write-avoidance rationale is void because a detections INSERT already happens per throttled request.
Failure scenario
Fresh 2.3.2 install, defaults: monitor_mode=true, rate_limit_enabled=true, rate_limit_requests=60, rate_limit_window=60, no tripwire hits. An aggressive third-party crawler (or an office NAT) issues 120 front-end requests/minute from one IP. WebDecoy_Rate_Limit_Rule returns THROTTLE (includes/class-webdecoy-rate-limit-rule.php:68) for ~60 requests/minute; handle_rule_decision() suppresses the 429 and, because of line 1484, records nothing. webdecoy_suppressed_actions stays empty, so render_state_notices() prints the $total === 0 branch (webdecoy.php:514): "Everything is detected and logged; nothing is blocked. No request has met the bar for enforcement yet." The owner reads that as "turning on enforcement changes nothing", disables monitor mode, and the plugin immediately starts serving 429s to that traffic. The comment's premise that THROTTLE "previously performed no database write on this path" is false: log_rule_violations() at webdecoy.php:1066 inserts a webdecoy_detections row for every violation, THROTTLE included, before handle_rule_decision() runs — so the flood it seeks to avoid already exists, and the skipped write is a single-row option UPDATE that is cheaper than the INSERT already performed.
Verified mechanism
Every link in the chain checks out on
fix/2.3.2-safety(HEAD 6f26683).monitor_mode => true,rate_limit_enabled => true,rate_limit_requests => 60,rate_limit_window => 60,rate_limit_algorithm => 'fixed',rate_limit_key => 'ip',rate_limit_dry_run => false. So on a fresh install the rate limiter is armed and non-dry-run.build_rule_engine()webdecoy.php:1208-1216 appendsWebDecoy_Rate_Limit_Rulewheneverrate_limit_enabled, so the engine is non-null even with nothing else configured.WebDecoy_Rate_Limit_Rule::evaluate()returns$this->action= THROTTLE (constructor line 68, result lines 90-104) oncecheck_and_increment()reportscurrent > limit. Fixed window keyed on the bare IP (line 135), so 120 req/min from one address yields ~60 THROTTLEs/min.RuleEngine::evaluate()(sdk/src/Rules/RuleEngine.php:69-96) records aViolationEventfor every non-ALLOW result — THROTTLE included — and makes it the deciding result.early_check()webdecoy.php:1066 callslog_rule_violations()for all violations, and that method does one$wpdb->insertintowebdecoy_detectionsper violation (webdecoy.php:1608-1616). Confirmed onmaintoo (git show main:webdecoy.php:894), so the comment's "it previously performed no database write on this path" is false. It is worse than the claim states: the default fixed-window limiter already does aSELECTplus anUPDATE/INSERTonwebdecoy_rate_limitsfor the same request (includes/class-webdecoy-rate-limiter.php:91-107). Three DB touches already occur; the one being avoided is a fourth.record_suppressed_action()exactly as claimed.webdecoy_suppressed_actionshas exactly one reader —render_state_notices()webdecoy.php:496 — verified by grep across the whole plugin. It sums onlycountand prints the literal string "No request has met the bar for enforcement yet." at line 512 when the sum is 0, or understates at line 509 when it is not.webdecoy_rate_limitsstores only the current window's counter and the row is deleted and re-inserted each new window (rate-limiter.php:102-107); it carries no history. Nothing inadmin/renders it — grep of admin/ forrate_limitreturns only the settings-form inputs.So the notice can state, on a default install, that nothing has met the bar for enforcement while ~60 requests/minute are being throttle-suppressed. The more common manifestation on a public site is the other branch: tripwire hits get counted so the total is non-zero, and the owner is shown a number that omits the throttles — which on a fast crawler or an office/CGNAT address is the dominant term by orders of magnitude. The owner reads "would have acted on 12 requests in 30 days", disables monitor mode, and the site starts serving 429s. Note also that the good-bot exemption is checked after the engine short-circuits (webdecoy.php:1094, reached only when the engine allows), so a fast Googlebot/Bingbot crawl is throttle-eligible — this is precisely the "blocked my customers/SEO" outcome the release exists to prevent, and it is the one class of suppressed action the counter cannot show.
The whole suppression mechanism (
monitor_mode,suppression_reason(),record_suppressed_action(),render_state_notices()) is new in this branch —git show main:webdecoy.php | grep monitor_modeis empty — so this is a new-in-branch defect, not inherited. No test asserts the exclusion (grep of tests/ forsuppressed_actionreturns nothing), so the fix is safe against the 75/75 suite.Two corrections to the claim's framing, neither of which changes the verdict: (a) the changelog points owners at the Detections page, and throttle violations do land there as rows flagged
rate-limit:60/60s_rule, so the data is not wholly invisible — but the notice's assertion is still false, which is the stated blocker criterion; (b) the fix is not simply deleting theif.$result->reasonfor the limiter embeds the varying counter ("Rate limit exceeded: 61/60 requests in 60s window", rate-limit-rule.php:94), so an unnormalized record would add a fresh key to$stats[$day]['by']on every throttled request and run thearsort+array_sliceat webdecoy.php:1449-1452 each time. The reason must be normalized to the stable rule name.Fix
webdecoy.php:1479-1489 — count THROTTLE, but with a stable reason key so the breakdown map cannot churn. Replace the comment and the
ifwith:$result->rulehere is the limiter'sgetName()("rate-limit:60/60s", class-webdecoy-rate-limit-rule.php:74), which is constant for a given configuration.Separately, while in this method: the docblock at webdecoy.php:1434 says "a single autoloaded counter", but line 1458 passes
falsefor autoload. Reword to "a single non-autoloaded counter" so the next reader does not go looking for an autoload problem.Blocks the 2.3.2 release. Refs #54.