security: trusted-proxy rate limiting, atomic restore, encrypted webhook secret#10
Merged
Merged
Conversation
…ook secret
Follow-ups from the security review, each verified in a live WP 7.0 install.
1. Trusted-proxy client IP (rate limiting was keyed on REMOTE_ADDR):
- New WP_Puller_Client_IP resolves the real client behind reverse proxies
and CDNs. Forwarding headers (CF-Connecting-IP, True-Client-IP,
X-Forwarded-For) are honoured ONLY when REMOTE_ADDR is itself a trusted
proxy, so they cannot be spoofed to evade the limiter.
- Cloudflare ranges supported out of the box (fetched weekly + hard-coded
fallback); loopback/RFC1918 covered for a site's own nginx/HAProxy.
- Extensible via `wp_puller_trusted_proxies` / `wp_puller_client_ip_headers`
for Sucuri, Akamai, custom load balancers, etc. IPv4 + IPv6 CIDR matching.
2. Atomic theme restore (restore_backup deleted the live theme before copying):
- Stage the restored copy in a dot-prefixed sibling dir, then swap via fast
renames with rollback. A failed restore can no longer brick the theme.
3. Webhook secret encrypted at rest:
- Stored via the existing WordPress salt-derived WP_Puller::encrypt() (the
same mechanism as the GitHub PAT). Reads are backward compatible with
legacy plaintext secrets, which are transparently upgraded on next save
or admin page view. The secret is still shown decrypted for GitHub setup.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Follow-ups from the live security review. Each change was verified against a real WordPress 7.0 / PHP 8.2 install (code-level harness + live HTTP).
1. Trusted-proxy client IP for rate limiting
The webhook rate limiter keyed on
REMOTE_ADDR, so behind a CDN/reverse proxy every visitor shared one bucket.WP_Puller_Client_IPresolves the real client IP. Forwarding headers (CF-Connecting-IP,True-Client-IP,X-Forwarded-For) are trusted only whenREMOTE_ADDRis itself a trusted proxy — so a direct attacker can't spoof them to dodge the limiter.wp_safe_remote_get+ hard-coded fallback). Loopback/RFC1918 covered for a site's own nginx/HAProxy.wp_puller_trusted_proxiesandwp_puller_client_ip_headersfilters. Full IPv4 + IPv6 CIDR matching.Verified: spoofed
X-Forwarded-For/CF-Connecting-IPfrom an untrusted source is ignored; Cloudflare/Akamai/private-proxy paths resolve correctly; rate limiter buckets per real client and can't be split by header spoofing.2. Atomic theme restore
restore_backup()deleted the live theme before copying the backup — a mid-copy failure left the site themeless.rename()s with rollback.Verified: happy path restores correctly; a forced failure (read-only parent) returns a
WP_Errorwith the live theme fully intact and no leftover temp dirs.3. Webhook secret encrypted at rest
Per review finding #1, but using the WordPress-idiomatic mechanism rather than anything custom: the existing salt-derived
WP_Puller::encrypt()(same as the GitHub PAT).v2:) on the next save or admin page view.Verified: legacy plaintext verifies; lazy migration to
v2:works and keeps verifying; fresh secret stored encrypted (raw DB value isv2:…, no plaintext); live signed webhook returns 200.Testing
php -lclean across the plugin; CI green.Notes
wp-puller.ziprebuild (handled separately, per CONTRIBUTING).@since 1.0.8.