Refactor/implement security measures rest api#4
Open
mvdhoek1 wants to merge 30 commits into
Open
Conversation
mvdhoek1
requested review from
Rovasch,
ShunLuk,
SimonvanWijhe,
Copilot,
dtakken,
ictbeheer and
rivanuff
There was a problem hiding this comment.
Pull request overview
This PR hardens the plugin’s REST translation endpoint by tightening authentication/authorization, adding rate limiting, and preventing unauthorized users from generating cached translations, while also adding an admin-side metabox UI for cache control and corresponding editor styles.
Changes:
- Updated REST API security model (nonce handling, origin validation) and introduced IP-based rate limiting with a capability-based bypass for authorized users.
- Refactored translation caching so storing cached translations is conditional (capability-driven), and added metabox controls to disable/clear cache on save.
- Added an editor stylesheet build entry and enqueued admin assets; committed
dist/output by removing it from.gitignore.
Reviewed changes
Copilot reviewed 10 out of 17 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| webpack.config.js | Adds a dedicated editor entry to build admin/editor CSS. |
| src/Services/TranslationService.php | Adds capability-driven caching toggle and a cached-translation helper. |
| src/Providers/RestAPIServiceProvider.php | Switches REST nonce verification to X-WP-Nonce/wp_rest. |
| src/Providers/MetaBoxServiceProvider.php | Refactors metabox registration/rendering; adds cache clear option; renames a public filter. |
| src/Providers/AssetsServiceProvider.php | Uses asset path helper for .asset.php, uses build dependencies, enqueues admin editor CSS. |
| src/helpers.php | Adds path helpers (ydpl_path, ydpl_asset_path) and updates docstrings. |
| src/Controllers/RestAPIController.php | Adds origin enforcement, rate limiting, and capability-gated caching behavior. |
| package-lock.json | Lockfile metadata/dep graph update (e.g., package name, ajv-formats requires). |
| languages/yard-deepl.pot | Regenerates POT with updated plugin header strings and new metabox strings. |
| languages/yard-deepl-nl_NL.po | Updates NL translations and syncs headers/strings with regenerated POT. |
| dist/editor.js | Built editor bundle artifact (currently empty output). |
| dist/editor.css | Built/minified editor CSS for metabox styling. |
| dist/editor.asset.php | Build metadata for the editor entry. |
| dist/editor-rtl.css | Built RTL CSS for the editor entry. |
| assets/css/editor.css | Source editor CSS for metabox styling. |
| .gitignore | Stops ignoring dist/ so built assets are committed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+46
to
+50
| $origin = $request->get_header( 'origin' ); | ||
|
|
||
| if ( is_null( $origin ) || home_url() !== $origin ) { | ||
| return $this->set_failure_response( 403, 'Invalid origin. Origin does not match the site URL.' ); | ||
| } |
Comment on lines
+63
to
+65
|
|
||
| // Apply rate limit check if object ID is absent or translation is not cached when an object ID is present. | ||
| if ( empty( $object_id ) || ! $this->service->object_has_cached_translation( (int) $object_id, $target_lang ) ) { |
Comment on lines
+121
to
125
|
|
||
| return $remote_address; | ||
| } | ||
|
|
||
| /** |
Comment on lines
+87
to
+90
| public function verify_nonce(): bool | ||
| { | ||
| return wp_verify_nonce( sanitize_text_field( wp_unslash( $_SERVER['HTTP_NONCE'] ?? '' ) ), YDPL_NONCE_REST_NAME ) || is_user_logged_in(); | ||
| $nonce = sanitize_text_field( wp_unslash( $_SERVER['HTTP_X_WP_NONCE'] ?? '' ) ); | ||
| return (bool) wp_verify_nonce( $nonce, 'wp_rest' ); |
|
|
||
| /** | ||
| * Add prefix for the given string. | ||
| * Generates a full plugin path by appending the given path to the base plugin URL. |
Comment on lines
+69
to
+76
| /** | ||
| * @since NEXT | ||
| */ | ||
| public function object_has_cached_translation( int $object_id, string $target_lang ): ?array | ||
| { | ||
| try { | ||
| return $this->repository->get_cached_translation( $object_id, $target_lang ); | ||
| } catch ( ObjectNotFoundException $e ) { |
Comment on lines
32
to
37
| add_meta_box( | ||
| 'yard-deepl', | ||
| __( 'Yard Deepl', 'yard-deepl' ), | ||
| array( $this, 'render_meta_box' ), | ||
| apply_filters( 'yard::deepl/disable_cache_metabox_post_types', array( 'page' ) ), | ||
| array( $this, 'render_meta_boxes' ), | ||
| apply_filters( 'yard::deepl/cache_metabox_post_types', array( 'page' ) ), | ||
| 'side', |
mvdhoek1
force-pushed
the
refactor/implement-security-measures-rest-api
branch
from
8e26c72 to
7189d14
Compare
| - `ydpl_supported_languages`: The list of languages supported for translation. | ||
| - `ydpl_api_request_nonce`: The nonce used for API validation. | ||
|
|
||
| When making requests to the API, ensure that the nonce is included in the request headers. The header should be named `nonce`, and it should contain the value of `ydpl_api_request_nonce`. |
| @@ -54,16 +62,20 @@ The API endpoints registered by this plugin are secured using a WordPress nonce. | |||
|
|
|||
| When making requests to the API, ensure that the nonce is included in the request headers. The header should be named `nonce`, and it should contain the value of `ydpl_api_request_nonce`. | |||
| { | ||
| add_meta_box( | ||
| 'yard-deepl', | ||
| __( 'Yard Deepl', 'yard-deepl' ), |
Comment on lines
+157
to
+162
| $post_types = apply_filters_deprecated( | ||
| 'yard::deepl/disable_cache_metabox_post_types', | ||
| array( array( 'page' ) ), | ||
| 'NEXT', | ||
| 'yard::deepl/cache_metabox_post_types' | ||
| ); |
Comment on lines
+42
to
45
| public function handle_translation_with_object_id( int $object_id, array $text, string $target_lang, bool $cache = false, ?array $cached_translation = null ): array | ||
| { | ||
| $cached_translation = $this->repository->get_cached_translation( $object_id, $target_lang ); | ||
| $cached_translation = $cached_translation ?? $this->get_cached_translation( $object_id, $target_lang ); | ||
|
|
Comment on lines
+118
to
+123
|
|
||
| if ( filter_var( $remote_address, FILTER_VALIDATE_IP ) === false ) { | ||
| return ''; | ||
| } | ||
|
|
||
| return $remote_address; |
Comment on lines
140
to
143
| && ( $home['port'] ?? null ) === ( $parsed['port'] ?? null ); | ||
| } | ||
|
|
||
| /** |
mvdhoek1
force-pushed
the
refactor/implement-security-measures-rest-api
branch
from
14f09b5 to
1936fd7
Compare
ictbeheer
reviewed
May 26, 2026
|
|
||
| // Are required by Deepl. | ||
| if ( empty( $text ) || empty( $target_lang ) ) { | ||
| if ( array() === $text || 1 > strlen( $target_lang ) ) { |
Member
There was a problem hiding this comment.
Dit kan ook in de validate_callback in register_rest_route()
… to yard::deepl/cache_metabox_post_types
mvdhoek1
force-pushed
the
refactor/implement-security-measures-rest-api
branch
from
1c39616 to
f4f3c38
Compare
ictbeheer
reviewed
May 26, 2026
| 'default' => 'NL', | ||
| 'required' => true, | ||
| 'validate_callback' => function ( $value, $request, $param ) { | ||
| return is_string( $value ) && strlen( $value ) > 0; |
Member
There was a problem hiding this comment.
Is die typecheck nodig? Dat doet WP toch al op basis van 'type'?
Je kan trouwens ook minItems => 1 opgeven, dan is de validate callback helemaal niet meer nodig:https://developer.wordpress.org/rest-api/extending-the-rest-api/schema/#arrays
Bij string kan je op die manier ook minLength opgeven https://developer.wordpress.org/rest-api/extending-the-rest-api/schema/#strings
mvdhoek1
force-pushed
the
refactor/implement-security-measures-rest-api
branch
from
f4f3c38 to
bbe4238
Compare
| $meta_keys = get_post_meta( $post_id ); | ||
|
|
||
| foreach ( $meta_keys as $key => $value ) { | ||
| if ( strpos( $key, '_translation_' ) === 0 ) { |
| $post_types = apply_filters_deprecated( | ||
| 'yard::deepl/disable_cache_metabox_post_types', | ||
| array( array( 'page' ) ), | ||
| 'NEXT', |
| $html .= '<div class="ydpl-metabox-row">'; | ||
| $html .= sprintf( '<p>%s</p>', esc_html__( 'Disable translation cache when this object contains dynamic content.', 'yard-deepl' ) ); | ||
| $html .= '<label for="ydpl_disable_deepl_translation_cache">'; | ||
| $html .= sprintf( '<input type="checkbox" name="ydpl_disable_deepl_translation_cache" id="ydpl_disable_deepl_translation_cache" value="1"%s />', esc_attr( checked( $cache_is_disabled, 1, false ) ) ); |
Comment on lines
+52
to
64
| $user_has_cache_capability = current_user_can( apply_filters( 'yard::deepl/cache_capability', 'edit_posts' ) ); | ||
| $cached_translation = ( 0 < $object_id ) ? ( $this->service->get_cached_translation( $object_id, $target_lang ) ?? array() ) : null; | ||
|
|
||
| // Apply rate limit check if object ID is absent or translation is not cached when an object ID is present. | ||
| if ( ! $cached_translation ) { | ||
| if ( $this->is_rate_limit_exceeded() && ! $user_has_cache_capability ) { | ||
| return $this->set_failure_response( 429, 'Rate limit exceeded.' ); | ||
| } | ||
| } | ||
|
|
||
| try { | ||
| $translation = $this->service->handle_translation( (int) $object_id, $text, $target_lang ); | ||
| $translation = $this->service->handle_translation( $object_id, $text, $target_lang, $user_has_cache_capability, $cached_translation ); | ||
|
|
Comment on lines
+107
to
+118
| $remote_address = $_SERVER['REMOTE_ADDR'] ?? ''; | ||
|
|
||
| if ( filter_var( $remote_address, FILTER_VALIDATE_IP ) === false ) { | ||
| return ''; | ||
| } | ||
|
|
||
| return $remote_address; | ||
| } | ||
|
|
||
| /** | ||
| * @since NEXT | ||
| */ |
| msgstr "Yard | Digital" | ||
| #: yard-deepl.php:11 | ||
| msgid "Yard | Digital Agency" | ||
| msgstr "" |
| 'default' => array(), | ||
| 'required' => true, | ||
| 'minItems' => 1, | ||
| 'sanitize_callback' => function ( $value, $request, $param ) { |
Comment on lines
40
to
47
| 'ydpl-main', | ||
| 'ydpl', | ||
| array( | ||
| 'ydpl_translate_post_id' => get_the_ID() ?: 0, | ||
| 'ydpl_translate_post_id' => get_the_ID() ?: get_queried_object_id() ?: 0, | ||
| 'ydpl_rest_translate_url' => esc_url_raw( rest_url( YDPL_API_NAMESPACE . '/translate' ) ), | ||
| 'ydpl_supported_languages' => $this->format_selected_supported_languages(), | ||
| 'ydpl_api_request_nonce' => wp_create_nonce( YDPL_NONCE_REST_NAME ), | ||
| 'ydpl_api_request_nonce' => wp_create_nonce( 'wp_rest' ), | ||
| ) |
Comment on lines
129
to
145
| @@ -107,7 +139,7 @@ private function should_save_metabox_values( int $post_id ): bool | |||
| } | |||
|
|
|||
| // Only allow updates for supported post types. | |||
| $post_types = apply_filters( 'yard::deepl/disable_cache_metabox_post_types', array( 'page' ) ); | |||
| $post_types = $this->get_cache_metabox_post_types(); | |||
| if ( ! in_array( get_post_type( $post_id ), $post_types, true ) ) { | |||
| return false; | |||
| } | |||
Comment on lines
79
to
87
| @@ -55,6 +86,30 @@ protected function translated_object_exists( string $object_id ): bool | |||
| return $object instanceof WP_Post; | |||
| } | |||
Comment on lines
+63
to
+71
| $post_modified = get_post_field( 'post_modified', $object_id ); | ||
| $all_meta = get_post_meta( $object_id ); | ||
| $cached = array(); | ||
|
|
||
| foreach ( $language_codes as $lang ) { | ||
| $translation_value = $all_meta[ "_translation_$lang" ][0] ?? null; | ||
| $translation_modified = $all_meta[ "_translation_modified_$lang" ][0] ?? null; | ||
|
|
||
| if ( $translation_value && $translation_modified && strtotime( $translation_modified ) >= strtotime( $post_modified ) ) { |
Comment on lines
58
to
64
| 'sanitize_callback' => function ( $value, $request, $param ) { | ||
| if ( ! is_array( $value ) ) { | ||
| return $value; | ||
| } | ||
|
|
||
| return array_map( 'sanitize_text_field', $value ); | ||
| }, |
Comment on lines
52
to
+60
| $translation = $this->handle_translation_without_object_id( $text, $target_lang ); | ||
|
|
||
| $this->repository->store_translation( $object_id, $target_lang, $translation ); | ||
| if ( ! $cache ) { | ||
| $this->repository->increment_uncached_request_count( $object_id, $target_lang ); | ||
| } | ||
|
|
||
| if ( $cache ) { | ||
| $this->repository->store_translation( $object_id, $target_lang, $translation ); | ||
| } |
Comment on lines
+70
to
+80
| $post_modified = get_post_field( 'post_modified', $object_id ); | ||
| $post_modified_timestamp = strtotime( $post_modified ); | ||
| $all_meta = get_post_meta( $object_id ); | ||
| $cached = array(); | ||
| $counts = array(); | ||
|
|
||
| foreach ( $language_codes as $lang ) { | ||
| $translation_value = $all_meta[ "_translation_$lang" ][0] ?? null; | ||
| $translation_modified = $all_meta[ "_translation_modified_$lang" ][0] ?? null; | ||
|
|
||
| if ( $translation_value && $translation_modified && strtotime( $translation_modified ) >= $post_modified_timestamp ) { |
Comment on lines
+92
to
+110
| protected function is_rate_limit_exceeded(): bool | ||
| { | ||
| $client_ip = $this->get_client_ip(); | ||
|
|
||
| if ( '' === $client_ip ) { | ||
| return true; | ||
| } | ||
|
|
||
| $transient_key = 'ydpl_rate_limit_' . hash_hmac( 'sha256', $client_ip, SECURE_AUTH_KEY ); | ||
| $request_count = (int) ( get_transient( $transient_key ) ?: 0 ); | ||
|
|
||
| if ( self::RATE_LIMIT <= $request_count ) { | ||
| return true; | ||
| } | ||
|
|
||
| set_transient( $transient_key, $request_count + 1, self::RATE_LIMIT_TIME_WINDOW_IN_SECONDS ); | ||
|
|
||
| return false; | ||
| } |
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.
Related: https://github.com/yardinternet/a11y-toolbar/pull/22
Make sure to merge both PR's simultaneously, otherwise the translations will fail for visitors as well.
Important fixes: