Implement PackRelay WordPress plugin#1
Conversation
Add REST API bridge for WPForms that enables external apps and mobile clients to submit form entries with Google reCAPTCHA v3 protection. - Plugin core: bootstrap, loader, settings, REST API endpoints - Form submission via wpforms()->entry->add() with field sanitization - reCAPTCHA v3 server-side verification with configurable threshold - Email notifications via wp_mail() with Reply-To support - WordPress Settings API admin page under Settings > PackRelay - CORS support with configurable allowed origins - Form ID allowlist for security - WordPress hooks/filters for extensibility - PHPUnit tests (45 tests, 115 assertions) with Brain Monkey - GitHub Actions CI/CD: test matrix, PR builds, release ZIP - React Native mobile integration guide Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
📦 Dev build ready! Download the plugin ZIP from the Actions artifacts. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThis pull request introduces PackRelay, a complete WordPress plugin that enables external apps and mobile clients to submit form entries to WPForms via REST API with Google reCAPTCHA v3 verification, email notifications, form allowlisting, CORS support, and a comprehensive admin settings interface. The plugin includes build workflows, documentation, extensive test coverage, and all necessary bootstrapping code. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as External Client
participant REST as PackRelay REST API
participant Captcha as reCAPTCHA Service
participant Entry as WPForms Entry
participant Notify as Email Notifier
Client->>REST: POST /submit/{form_id}<br/>with token & fields
REST->>REST: Validate form_id in allowlist
REST->>Captcha: Verify token
Captcha-->>REST: Response with score
REST->>REST: Check score vs threshold
REST->>REST: Validate required fields
REST->>REST: Sanitize field values
REST->>Entry: Create WPForms entry
Entry-->>REST: entry_id
REST->>Notify: Send notification email
Notify-->>REST: Success/Failure
REST-->>Client: 200 + entry_id
sequenceDiagram
participant Admin as Admin User
participant Dashboard as WordPress Dashboard
participant Settings as PackRelay Settings
participant DB as WordPress Database
Admin->>Dashboard: Navigate to PackRelay Settings
Dashboard->>Settings: Load settings page
Settings->>DB: get_option(packrelay_settings)
DB-->>Settings: Current settings
Settings-->>Dashboard: Render form with defaults
Admin->>Dashboard: Enter credentials & save
Dashboard->>Settings: sanitize_settings()
Settings->>Settings: Validate & clamp values
Settings->>DB: update_option(packrelay_settings)
DB-->>Settings: Saved
Settings-->>Dashboard: Success message
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (9)
includes/class-packrelay-settings.php (1)
144-155: Default CORS behavior of "allow all" is permissive.The "Leave blank to allow all" default for allowed origins means a fresh install with no configuration will respond with permissive CORS headers to any origin. Consider defaulting to a restrictive posture (e.g., same-origin only) and requiring explicit opt-in for cross-origin access, especially since this plugin handles form submissions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@includes/class-packrelay-settings.php` around lines 144 - 155, The settings field 'allowed_origins' currently implies "Leave blank to allow all" which yields permissive CORS; change the default posture to restrictive by updating the add_settings_field registration and associated handling so a blank or unset 'allowed_origins' is treated as same-origin only (no wildcard allow), require explicit comma-separated origins to enable cross-origin, and update the field description to reflect this opt-in behavior; also adjust the render_text_field and any CORS header logic that reads 'allowed_origins' (look for render_text_field, allowed_origins, and the code that emits CORS headers) to validate/sanitize the list and deny cross-origin when the list is empty or invalid.CLAUDE.md (1)
34-45: Add a language identifier to the fenced code block.The static analysis tool flagged this code block for missing a language specifier. Since this depicts an ASCII flow diagram, you could use
textorplaintext.-``` +```text POST /packrelay/v1/submit/{form_id}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` around lines 34 - 45, The fenced code block containing the ASCII flow diagram (the block starting with "POST /packrelay/v1/submit/{form_id}" and the sequence lines referencing REST_API::handle_submit, is_form_allowed, get_wpforms_form, ReCaptcha::verify, Entry::create, Notification::send, do_action('packrelay_entry_created'), return WP_REST_Response) needs a language identifier; update the opening fence from ``` to ```text (or ```plaintext) so the block is marked as plain text for the linter..github/workflows/test.yml (1)
28-41: Consider caching Composer dependencies to speed up CI.Adding a Composer cache step would reduce install times across the matrix, especially with 4 PHP versions.
♻️ Example cache step
- name: Setup PHP uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php }} coverage: none + - name: Cache Composer packages + uses: actions/cache@v4 + with: + path: vendor + key: php-${{ matrix.php }}-composer-${{ hashFiles('composer.lock') }} + restore-keys: php-${{ matrix.php }}-composer- + - name: Install dependencies run: composer install --prefer-dist --no-progress🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 28 - 41, Add a Composer cache step to the workflow to speed up the "Install dependencies" run: insert a step named like "Cache Composer dependencies" (before the "Install dependencies" step) using actions/cache to cache vendor and Composer cache directories, with a key that includes the composer.lock hash and the matrix PHP version (e.g., key derived from hashFiles('**/composer.lock') and matrix.php) and restore-keys fallback so composer install can reuse cached packages across matrix jobs; reference the existing step names "Install dependencies" and the composer install command to locate where to place this new cache step.tests/EntryTest.php (1)
43-47: Unused$hookparameter inandReturnUsingclosures.PHPMD flags the unused
$hookparameter in theapply_filtersmock callbacks at lines 45, 76, and 110. Since only$fieldsis needed, you can prefix with underscore to signal intent.♻️ Proposed fix (apply to all three occurrences)
- ->andReturnUsing( function ( $hook, $fields ) { + ->andReturnUsing( function ( $_hook, $fields ) { return $fields; } );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/EntryTest.php` around lines 43 - 47, The mock callbacks for Functions\expect('apply_filters')->andReturnUsing currently declare an unused $hook parameter which PHPMD flags; update the closure signatures used in apply_filters mocks (the andReturnUsing closures) to prefix the unused parameter with an underscore (e.g., $_hook) or replace it with $_ to indicate it is intentionally unused, and apply this change to all three occurrences of the apply_filters mock callbacks so only $fields remains used inside the closures.includes/class-packrelay-recaptcha.php (3)
55-64: Consider adding a timeout to the reCAPTCHA verification request.The
wp_remote_postcall uses WordPress's default timeout (5 seconds). For a request that blocks form submission, you may want to set an explicit, shorter timeout to avoid hanging the user's request if Google's API is slow.Optional: explicit timeout
$response = wp_remote_post( self::VERIFY_URL, array( + 'timeout' => 5, 'body' => array( 'secret' => $secret_key, 'response' => $token, 'remoteip' => $ip, ), ) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@includes/class-packrelay-recaptcha.php` around lines 55 - 64, The wp_remote_post call that sends the reCAPTCHA verification to self::VERIFY_URL (where $secret_key, $token, $ip are used) should include an explicit timeout to avoid blocking form submission; update the request args passed to wp_remote_post in the class-packrelay-recaptcha.php method that builds $response to include a suitable 'timeout' value (e.g. 2–3 seconds) alongside the existing 'body' array so the request fails fast if Google's API is slow.
35-118: No logging on verification failures — consider adding debug-level logging.When reCAPTCHA verification fails (network error, non-200 response, low score), there's no server-side logging. This can make debugging production issues difficult. Consider adding
error_log()or using WordPress'sWP_DEBUG_LOGfor failure paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@includes/class-packrelay-recaptcha.php` around lines 35 - 118, Add debug-level logging to PackRelay_ReCaptcha::verify to record failure details for each rejection path (missing token/secret, is_wp_error($response) including $response->get_error_message(), non-200 status with the status code and response body, json_decode failures or body['success'] false including the raw body, and low-score rejections including the score and threshold). Use a WP logging approach (error_log or WP_DEBUG_LOG via if ( defined('WP_DEBUG') && WP_DEBUG ) ), include identifying context like form_id, VERIFY_URL and the secret/threshold keys from PackRelay_Settings to aid debugging, and avoid logging sensitive full secret key (log masked or indicate presence).
93-93: Null coalescing on$settings['recaptcha_threshold']is redundant.
PackRelay_Settings::get_settings()already merges stored options with defaults (which include'recaptcha_threshold' => 0.5), so this key will always be present. Not harmful, just unnecessary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@includes/class-packrelay-recaptcha.php` at line 93, The code uses a redundant null coalescing when reading recaptcha_threshold; remove the unnecessary fallback so that $threshold is set directly from the settings array (e.g. $threshold = floatval( $settings['recaptcha_threshold'] );), since PackRelay_Settings::get_settings() already supplies the default; update the line that assigns $threshold in the class-packrelay-recaptcha.php (referencing $threshold and the recaptcha_threshold key) accordingly.MOBILE_INTEGRATION.md (1)
74-81:JSON.parsewithout try/catch in example code.If the WebView sends malformed data,
JSON.parsewill throw and crash the handler. Since this is user-facing example code, wrapping it defensively would be good practice.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MOBILE_INTEGRATION.md` around lines 74 - 81, The onMessage handler uses JSON.parse(event.nativeEvent.data) without error handling, so a malformed payload will throw and crash; update the useCallback onMessage function to wrap JSON.parse in a try/catch, safely handle parse failures (e.g., call onError with the parse error if provided, or return early), and only proceed to call onToken or the error branch when parsing succeeds—refer to onMessage, useCallback, event.nativeEvent.data, onToken, and onError when making the change.includes/class-packrelay-rest-api.php (1)
51-55: Consider constructor injection for testability.Hard-coding dependency instantiation couples this class to its collaborators. Accepting optional constructor parameters (defaulting to
new PackRelay_ReCaptcha()etc.) would make unit testing simpler without changing existing call sites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@includes/class-packrelay-rest-api.php` around lines 51 - 55, The constructor currently hardcodes dependencies by instantiating PackRelay_ReCaptcha, PackRelay_Entry, and PackRelay_Notification inside __construct, which hinders testability; change __construct to accept optional parameters (e.g., $recaptcha = null, $entry = null, $notification = null) and assign $this->recaptcha = $recaptcha ?: new PackRelay_ReCaptcha(), $this->entry = $entry ?: new PackRelay_Entry(), $this->notification = $notification ?: new PackRelay_Notification() so existing call sites remain unchanged but tests can inject mocks via the constructor (refer to the __construct method and the properties $recaptcha, $entry, $notification).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-zip.yml:
- Around line 7-10: The workflow job "build" lacks a permissions block so steps
that comment on PRs (the "Comment on PR" step) may be denied; add a permissions
declaration to the build job (e.g., permissions: contents: write) so the job has
the necessary write access for commenting on issues/pull-requests, ensuring the
"build" job's permissions allow the PR-commenting step to run without 403
errors.
In `@CLAUDE.md`:
- Around line 88-96: Update the examples in CLAUDE.md to use the actual
PascalCase test filenames and glob pattern: change references like
tests/test-recaptcha.php and tests/test-*.php to tests/ReCaptchaTest.php,
tests/EntryTest.php (as examples) and the pattern tests/*Test.php, and update
the phpunit command examples accordingly (e.g., vendor/bin/phpunit
tests/ReCaptchaTest.php and vendor/bin/phpunit --filter testName) so the docs
match the real test filenames and will run successfully.
In `@includes/class-packrelay-entry.php`:
- Around line 52-55: The code trusts the client-controlled X-Forwarded-For
header when setting $ip via $request->get_header('X-Forwarded-For') which is
spoofable and may contain a comma-separated list; change the logic in the method
that sets $ip to instead use $_SERVER['REMOTE_ADDR'] as the primary reliable
source, or if proxy support is required, parse the header to extract a single
candidate IP (first or last element), validate it with filter_var($ip,
FILTER_VALIDATE_IP) before accepting it, and only fall back to
sanitize_text_field when you have a validated single IP; update the usage of
sanitize_text_field accordingly so you never store an unvalidated or multi-value
header string.
In `@includes/class-packrelay-rest-api.php`:
- Around line 278-285: Don't default to Access-Control-Allow-Origin: * when
get_allowed_origins() returns empty; instead treat empty as deny-by-default:
remove the header('Access-Control-Allow-Origin: *') branch and, if
$allowed_origins is empty, do not set any CORS header and return a 403/deny
response (or a WP_Error) for cross-origin requests; keep the existing elseif
branch that sets header to the specific $origin only when in_array($origin,
$allowed_origins, true). Update any admin-facing messaging elsewhere to explain
that at least one origin must be configured if cross-origin access is required.
- Around line 299-302: The is_form_allowed method reads
$settings['allowed_form_ids'] without a fallback which can pass null into
explode; update the call in is_form_allowed (where $allowed_ids is built) to use
a null-coalescing default like $settings['allowed_form_ids'] ?? '' before
trim/explode so explode always receives a string (mirroring the approach used in
get_allowed_origins), keeping the rest of the array_map/array_filter logic
intact.
- Around line 134-148: The code is improperly trusting the entire
X-Forwarded-For header and passing it to $this->recaptcha->verify, which both
breaks verification (comma-separated list) and is spoofable; change the $ip
determination in the handler that uses $request->get_param and
$request->get_header so that you prefer $_SERVER['REMOTE_ADDR'] by default and
only use X-Forwarded-For when a trusted-proxy/config flag is enabled, and when
used extract the leftmost IP from the comma-separated value, trim it and
validate it with filter_var(..., FILTER_VALIDATE_IP) before passing $ip to
$this->recaptcha->verify; keep the error_response and recaptcha->verify call
unchanged except for using the sanitized single IP.
In `@MOBILE_INTEGRATION.md`:
- Around line 142-159: submitForm currently returns response.json() without
checking HTTP status, causing silent failures unlike getFormFields; update
submitForm to check response.ok after fetch and if false read the error body
(e.g., await response.text() or await response.json()) and throw a new Error
containing the response.status and the error body, otherwise return await
response.json(); reference the submitForm function and mirror the error-handling
pattern used in getFormFields to keep behavior consistent.
- Around line 95-97: Replace the fictional package name
"@anthropic/recaptcha-react-native" with a real, published alternative (choose
one such as "@haskkor/react-native-recaptchav3",
"react-native-google-recaptcha-v3", or "react-native-recaptcha-that-works") in
the "Option B" section; update the sentence to reference the chosen package name
and, if present, adjust any import or usage examples to match that package's
public API so readers can install and use the native reCAPTCHA wrapper.
---
Nitpick comments:
In @.github/workflows/test.yml:
- Around line 28-41: Add a Composer cache step to the workflow to speed up the
"Install dependencies" run: insert a step named like "Cache Composer
dependencies" (before the "Install dependencies" step) using actions/cache to
cache vendor and Composer cache directories, with a key that includes the
composer.lock hash and the matrix PHP version (e.g., key derived from
hashFiles('**/composer.lock') and matrix.php) and restore-keys fallback so
composer install can reuse cached packages across matrix jobs; reference the
existing step names "Install dependencies" and the composer install command to
locate where to place this new cache step.
In `@CLAUDE.md`:
- Around line 34-45: The fenced code block containing the ASCII flow diagram
(the block starting with "POST /packrelay/v1/submit/{form_id}" and the sequence
lines referencing REST_API::handle_submit, is_form_allowed, get_wpforms_form,
ReCaptcha::verify, Entry::create, Notification::send,
do_action('packrelay_entry_created'), return WP_REST_Response) needs a language
identifier; update the opening fence from ``` to ```text (or ```plaintext) so
the block is marked as plain text for the linter.
In `@includes/class-packrelay-recaptcha.php`:
- Around line 55-64: The wp_remote_post call that sends the reCAPTCHA
verification to self::VERIFY_URL (where $secret_key, $token, $ip are used)
should include an explicit timeout to avoid blocking form submission; update the
request args passed to wp_remote_post in the class-packrelay-recaptcha.php
method that builds $response to include a suitable 'timeout' value (e.g. 2–3
seconds) alongside the existing 'body' array so the request fails fast if
Google's API is slow.
- Around line 35-118: Add debug-level logging to PackRelay_ReCaptcha::verify to
record failure details for each rejection path (missing token/secret,
is_wp_error($response) including $response->get_error_message(), non-200 status
with the status code and response body, json_decode failures or body['success']
false including the raw body, and low-score rejections including the score and
threshold). Use a WP logging approach (error_log or WP_DEBUG_LOG via if (
defined('WP_DEBUG') && WP_DEBUG ) ), include identifying context like form_id,
VERIFY_URL and the secret/threshold keys from PackRelay_Settings to aid
debugging, and avoid logging sensitive full secret key (log masked or indicate
presence).
- Line 93: The code uses a redundant null coalescing when reading
recaptcha_threshold; remove the unnecessary fallback so that $threshold is set
directly from the settings array (e.g. $threshold = floatval(
$settings['recaptcha_threshold'] );), since PackRelay_Settings::get_settings()
already supplies the default; update the line that assigns $threshold in the
class-packrelay-recaptcha.php (referencing $threshold and the
recaptcha_threshold key) accordingly.
In `@includes/class-packrelay-rest-api.php`:
- Around line 51-55: The constructor currently hardcodes dependencies by
instantiating PackRelay_ReCaptcha, PackRelay_Entry, and PackRelay_Notification
inside __construct, which hinders testability; change __construct to accept
optional parameters (e.g., $recaptcha = null, $entry = null, $notification =
null) and assign $this->recaptcha = $recaptcha ?: new PackRelay_ReCaptcha(),
$this->entry = $entry ?: new PackRelay_Entry(), $this->notification =
$notification ?: new PackRelay_Notification() so existing call sites remain
unchanged but tests can inject mocks via the constructor (refer to the
__construct method and the properties $recaptcha, $entry, $notification).
In `@includes/class-packrelay-settings.php`:
- Around line 144-155: The settings field 'allowed_origins' currently implies
"Leave blank to allow all" which yields permissive CORS; change the default
posture to restrictive by updating the add_settings_field registration and
associated handling so a blank or unset 'allowed_origins' is treated as
same-origin only (no wildcard allow), require explicit comma-separated origins
to enable cross-origin, and update the field description to reflect this opt-in
behavior; also adjust the render_text_field and any CORS header logic that reads
'allowed_origins' (look for render_text_field, allowed_origins, and the code
that emits CORS headers) to validate/sanitize the list and deny cross-origin
when the list is empty or invalid.
In `@MOBILE_INTEGRATION.md`:
- Around line 74-81: The onMessage handler uses
JSON.parse(event.nativeEvent.data) without error handling, so a malformed
payload will throw and crash; update the useCallback onMessage function to wrap
JSON.parse in a try/catch, safely handle parse failures (e.g., call onError with
the parse error if provided, or return early), and only proceed to call onToken
or the error branch when parsing succeeds—refer to onMessage, useCallback,
event.nativeEvent.data, onToken, and onError when making the change.
In `@tests/EntryTest.php`:
- Around line 43-47: The mock callbacks for
Functions\expect('apply_filters')->andReturnUsing currently declare an unused
$hook parameter which PHPMD flags; update the closure signatures used in
apply_filters mocks (the andReturnUsing closures) to prefix the unused parameter
with an underscore (e.g., $_hook) or replace it with $_ to indicate it is
intentionally unused, and apply this change to all three occurrences of the
apply_filters mock callbacks so only $fields remains used inside the closures.
Security: - Fix IP handling: use REMOTE_ADDR as primary, parse and validate X-Forwarded-For first entry with filter_var(FILTER_VALIDATE_IP) - CORS deny-by-default: empty allowed_origins no longer sets wildcard - Add null coalescing on allowed_form_ids in is_form_allowed() - Add 3s timeout to reCAPTCHA wp_remote_post call CI: - Add permissions block to build-zip workflow for PR commenting - Add Composer dependency caching to test workflow - Bump PHP minimum to 8.1 (PHPUnit 10 requires 8.1+), add PHP 8.4 - Update composer.json, plugin header, and readme.txt accordingly Code quality: - Accept optional constructor args in REST_API for testability - Remove redundant ?? 0.5 fallback in recaptcha threshold - Fix unused $hook param in EntryTest closures Docs: - Fix CLAUDE.md test filenames to match PascalCase (*Test.php) - Add language identifier to flow diagram code fence - Fix MOBILE_INTEGRATION.md: add response.ok check to submitForm, try/catch on JSON.parse in onMessage, replace fictional package name - Update allowed_origins description to reflect deny-by-default Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
📦 Dev build ready! Download the plugin ZIP from the Actions artifacts. |
Summary
wp_mail()with Reply-To supportTest plan
composer installsucceedsmake test— all 45 tests passmake zip— produces clean production ZIP🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Chores