Refactor/implement security measures rest api#4
Open
mvdhoek1 wants to merge 8 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', |
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:
To-do:
Clarify in the README's (.md and .txt) that the caching mechanism only works for logged-in users with the edit_posts capability. This resolves an issue where malicious visitors could generate translation cache entries by POSTing invalid translations, without verifying whether the submitted translations actually matched strings requiring translation.