Connectors: Move API key validation and masking to REST dispatch level#76327
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @Copilot. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Flaky tests detected in 8d7a61c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/22901770952
|
There was a problem hiding this comment.
Pull request overview
This PR moves connector API key validation and masking out of option-level hooks and into a rest_post_dispatch filter targeting the /wp/v2/settings endpoint, aiming to ensure raw keys are never returned by REST while still validating updates.
Changes:
- Replaces per-setting
sanitize_callbackvalidation with REST dispatch-time validation that clears invalid keys. - Replaces option-level masking (
option_{name}filter) with REST response masking for/wp/v2/settings. - Removes the helper used to bypass option masking and simplifies AI client initialization to read options directly.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $is_update = 'POST' === $request->get_method() || 'PUT' === $request->get_method(); | ||
|
|
||
| foreach ( _gutenberg_get_connector_settings() as $connector_id => $connector_data ) { | ||
| $auth = $connector_data['authentication']; | ||
| if ( 'api_key' !== $auth['method'] || empty( $auth['setting_name'] ) ) { |
There was a problem hiding this comment.
_gutenberg_get_connector_settings() does heavy work (e.g., get_plugins() and building install/activation status) and is now called for every /wp/v2/settings response. This can add significant overhead to unrelated settings requests. Consider caching the connector list (static memoization) or adding a lightweight helper that only returns { connector_id => setting_name } for API-key connectors and avoid the plugin scan in this dispatch filter.
| // On update, validate the key before masking. | ||
| if ( $is_update && is_string( $value ) && '' !== $value ) { | ||
| if ( true !== _gutenberg_is_ai_api_key_valid( $value, $connector_id ) ) { | ||
| update_option( $setting_name, '' ); | ||
| $data[ $setting_name ] = ''; | ||
| continue; |
There was a problem hiding this comment.
The invalid-key path calls update_option( $setting_name, '' ) from rest_post_dispatch, which means the option has already been updated once by the Settings controller before this filter runs. That results in two option updates (and two update_option_* action runs) per invalid save, and transiently stores the invalid key during the request. If other hooks react to the first update, this can cause unintended side effects; consider validating earlier in the request lifecycle (e.g., a REST pre-insert hook / sanitize callback) so the invalid value is never persisted.
There was a problem hiding this comment.
I think it's a good trade-off as is, because we need a way to confirm whether the API key is valid by setting it for the AI provider. @jorgefilipecosta, can you think of something more lightweight here?
| /** | ||
| * Masks and validates connector API keys in REST responses. | ||
| * | ||
| * On every `/wp/v2/settings` response, masks connector API key values so raw | ||
| * keys are never exposed via the REST API. | ||
| * | ||
| * On POST requests, validates each updated key against the provider before | ||
| * masking. If validation fails, the key is reverted to an empty string. | ||
| * | ||
| * @access private | ||
| * | ||
| * @param WP_REST_Response $response The response object. | ||
| * @param WP_REST_Server $server The server instance. | ||
| * @param WP_REST_Request $request The request object. | ||
| * @return WP_REST_Response The modified response with masked/validated keys. | ||
| */ | ||
| function _gutenberg_connectors_rest_settings_dispatch( WP_REST_Response $response, WP_REST_Server $server, WP_REST_Request $request ): WP_REST_Response { | ||
| if ( '/wp/v2/settings' !== $request->get_route() ) { | ||
| return $response; | ||
| } | ||
|
|
||
| if ( ! class_exists( '\WordPress\AiClient\AiClient' ) ) { | ||
| return $response; | ||
| } | ||
|
|
||
| $data = $response->get_data(); | ||
| if ( ! is_array( $data ) ) { | ||
| return $response; | ||
| } | ||
|
|
||
| $is_update = 'POST' === $request->get_method() || 'PUT' === $request->get_method(); | ||
|
|
||
| foreach ( _gutenberg_get_connector_settings() as $connector_id => $connector_data ) { | ||
| $auth = $connector_data['authentication']; | ||
| if ( 'api_key' !== $auth['method'] || empty( $auth['setting_name'] ) ) { | ||
| continue; | ||
| } | ||
|
|
||
| $setting_name = $auth['setting_name']; | ||
| if ( ! array_key_exists( $setting_name, $data ) ) { | ||
| continue; | ||
| } | ||
|
|
||
| $value = $data[ $setting_name ]; | ||
|
|
||
| // On update, validate the key before masking. | ||
| if ( $is_update && is_string( $value ) && '' !== $value ) { | ||
| if ( true !== _gutenberg_is_ai_api_key_valid( $value, $connector_id ) ) { | ||
| update_option( $setting_name, '' ); | ||
| $data[ $setting_name ] = ''; | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| // Mask the key in the response. | ||
| if ( is_string( $value ) && '' !== $value ) { | ||
| $data[ $setting_name ] = _gutenberg_mask_api_key( $value ); | ||
| } | ||
| } |
There was a problem hiding this comment.
This change introduces new REST-layer behavior (masking values on GET and reverting invalid keys on update) but there don’t appear to be E2E/unit tests covering these cases. Since the repo already has e2e coverage for the Connectors admin page, please add a test that asserts /wp/v2/settings?_fields=<setting> returns a masked value and that submitting an invalid key results in the option being cleared (and the UI surfacing the failure).
There was a problem hiding this comment.
That'd be a good follow-up after RC1. In particular, we can add e2e test that installs a plugin with a mocked AI provider to test the entire flow with API key management in the UI.
This comment was marked as resolved.
This comment was marked as resolved.
gziolo
left a comment
There was a problem hiding this comment.
The refactoring is clean and correct. The main improvement is that get_option() no longer returns masked values, eliminating the need for the _gutenberg_get_real_api_key workaround.
Two minor observations (not blockers):
-
Transient validation failures could clear valid keys. If
_gutenberg_is_ai_api_key_valid()returnsnull(network error), the key is reverted to empty. This matches the previous behavior though, so it's not a regression. -
Brief window with invalid key in DB on POST: The settings endpoint saves the value first, then the
rest_post_dispatchfilter validates and reverts if invalid. There's a brief moment where an invalid key exists in the DB. Low risk in practice since the revert happens in the same request.
It was also raised in Copilot's review at https://github.com/WordPress/gutenberg/pull/76327/changes#r2909665621.
Two potential follow-ups:
_gutenberg_get_connector_settings()might benefit from some optimizations as noted in https://github.com/WordPress/gutenberg/pull/76327/changes#r2909665610.- new e2e tests should be now possible to cover the editing flow as outlined in https://github.com/WordPress/gutenberg/pull/76327/changes#r2910198612
Move both concerns out of the options layer (sanitize_callback and
option_{name} filter) into a single rest_post_dispatch filter. This
removes the need for the _gutenberg_get_real_api_key workaround and
keeps raw keys out of REST responses without affecting internal
get_option calls.
…76348) * Initial plan * Fix docblock to mention both POST and PUT methods for API key validation Co-authored-by: gziolo <699132+gziolo@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: gziolo <699132+gziolo@users.noreply.github.com>
5fc0290 to
8d7a61c
Compare
#76327) Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org>
|
I just cherry-picked this PR to the wp/7.0 branch to get it included in the next release: 57c8a56 |
Add `_wp_connectors_get_api_key_source()` to detect whether an API key comes from an environment variable, PHP constant, or the database. This enables the UI to show the key source and hide the remove button for externally configured keys. Refactor API key validation and masking from `sanitize_callback` and `option_` filters into a single `rest_post_dispatch` handler (`_wp_connectors_rest_settings_dispatch`). This ensures raw keys are never exposed via the REST API and simplifies the validation flow. Enrich `_wp_connectors_get_connector_settings()` with plugin installation/activation status and static memoization. Update `_wp_connectors_get_connector_script_module_data()` to expose `keySource`, `isConnected`, `logoUrl`, and plugin status to the admin. Backports WordPress/gutenberg#76266 Backports WordPress/gutenberg#76327
Add `_wp_connectors_get_api_key_source()` to detect whether an API key comes from an environment variable, PHP constant, or the database. This enables the UI to show the key source and hide the remove button for externally configured keys. Refactor API key validation and masking from `sanitize_callback` and `option_` filters into a single `rest_post_dispatch` handler (`_wp_connectors_rest_settings_dispatch`). This ensures raw keys are never exposed via the REST API and simplifies the validation flow. Enrich `_wp_connectors_get_connector_settings()` with plugin installation/activation status and static memoization. Update `_wp_connectors_get_connector_script_module_data()` to expose `keySource`, `isConnected`, `logoUrl`, and plugin status to the admin. Backports WordPress/gutenberg#76266 Backports WordPress/gutenberg#76327
Add `_wp_connectors_get_api_key_source()` to detect whether an API key comes from an environment variable, PHP constant, or the database. This enables the UI to show the key source and hide the remove button for externally configured keys. Refactor API key validation and masking from `sanitize_callback` and `option_` filters into a single `rest_post_dispatch` handler (`_wp_connectors_rest_settings_dispatch`). This ensures raw keys are never exposed via the REST API and simplifies the validation flow. Enrich `_wp_connectors_get_connector_settings()` with plugin installation/activation status and static memoization. Update `_wp_connectors_get_connector_script_module_data()` to expose `keySource`, `isConnected`, `logoUrl`, and plugin status to the admin. Backports WordPress/gutenberg#76266 Backports WordPress/gutenberg#76327 updates include ref update
Add `_wp_connectors_get_api_key_source()` to detect whether an API key comes from an environment variable, PHP constant, or the database. This enables the UI to show the key source and hide the remove button for externally configured keys. Refactor API key validation and masking from `sanitize_callback` and `option_` filters into a single `rest_post_dispatch` handler (`_wp_connectors_rest_settings_dispatch`). This ensures raw keys are never exposed via the REST API and simplifies the validation flow. Enrich `_wp_connectors_get_connector_settings()` with plugin installation/activation status and static memoization. Update `_wp_connectors_get_connector_script_module_data()` to expose `keySource`, `isConnected`, `logoUrl`, and plugin status to the admin. Backports WordPress/gutenberg#76266 Backports WordPress/gutenberg#76327 updates include ref update
Add `_wp_connectors_get_api_key_source()` to detect whether an API key comes from an environment variable, PHP constant, or the database. This enables the UI to show the key source and hide the remove button for externally configured keys. Refactor API key validation and masking from `sanitize_callback` and `option_` filters into a single `rest_post_dispatch` handler (`_wp_connectors_rest_settings_dispatch`). This ensures raw keys are never exposed via the REST API and simplifies the validation flow. Enrich `_wp_connectors_get_connector_settings()` with plugin installation/activation status and static memoization. Update `_wp_connectors_get_connector_script_module_data()` to expose `keySource`, `isConnected`, `logoUrl`, and plugin status to the admin. Backports WordPress/gutenberg#76266 Backports WordPress/gutenberg#76327 updates include ref update
Add `_wp_connectors_get_api_key_source()` to detect whether an API key comes from an environment variable, PHP constant, or the database. This enables the UI to show the key source and hide the remove button for externally configured keys. Refactor API key validation and masking from `sanitize_callback` and `option_` filters into a single `rest_post_dispatch` handler (`_wp_connectors_rest_settings_dispatch`). This ensures raw keys are never exposed via the REST API and simplifies the validation flow. Enrich `_wp_connectors_get_connector_settings()` with plugin installation/activation status and static memoization. Update `_wp_connectors_get_connector_script_module_data()` to expose `keySource`, `isConnected`, `logoUrl`, and plugin status to the admin. Backports WordPress/gutenberg#76266 Backports WordPress/gutenberg#76327 updates include ref update # Conflicts: # src/wp-includes/connectors.php
…masking. Add `_wp_connectors_get_api_key_source()` to detect whether an API key is configured via environment variable, PHP constant, or database. The UI uses this to show the key source and hide "Remove and replace" for externally configured keys. Replace `_wp_connectors_validate_keys_in_rest()` and `_wp_connectors_get_real_api_key()` with a single `rest_post_dispatch` handler, `_wp_connectors_rest_settings_dispatch()`, that masks keys in all `/wp/v2/settings` responses and validates on POST/PUT, reverting invalid keys. Simplify `_wp_register_default_connector_settings()` by replacing the closure-based `sanitize_callback` and `option_` mask filter with plain `sanitize_text_field`, since masking is now handled at the REST layer. Enrich `_wp_connectors_get_connector_script_module_data()` to expose `keySource`, `isConnected`, `logoUrl`, and plugin `isInstalled` / `isActivated` status to the admin screen. Update `_wp_connectors_pass_default_keys_to_ai_client()` to skip keys sourced from environment variables or constants and read the database directly via `get_option()`. Set `_wp_connectors_init` priority to 15 so the registry is ready before settings are registered at priority 20. Backports WordPress/gutenberg#76266. Backports WordPress/gutenberg#76327. Props jorgefilipecosta, gziolo, swissspidy, flixos90. Fixes #64819. git-svn-id: https://develop.svn.wordpress.org/trunk@61985 602fd350-edb4-49c9-b593-d223f7449a82
…masking. Add `_wp_connectors_get_api_key_source()` to detect whether an API key is configured via environment variable, PHP constant, or database. The UI uses this to show the key source and hide "Remove and replace" for externally configured keys. Replace `_wp_connectors_validate_keys_in_rest()` and `_wp_connectors_get_real_api_key()` with a single `rest_post_dispatch` handler, `_wp_connectors_rest_settings_dispatch()`, that masks keys in all `/wp/v2/settings` responses and validates on POST/PUT, reverting invalid keys. Simplify `_wp_register_default_connector_settings()` by replacing the closure-based `sanitize_callback` and `option_` mask filter with plain `sanitize_text_field`, since masking is now handled at the REST layer. Enrich `_wp_connectors_get_connector_script_module_data()` to expose `keySource`, `isConnected`, `logoUrl`, and plugin `isInstalled` / `isActivated` status to the admin screen. Update `_wp_connectors_pass_default_keys_to_ai_client()` to skip keys sourced from environment variables or constants and read the database directly via `get_option()`. Set `_wp_connectors_init` priority to 15 so the registry is ready before settings are registered at priority 20. Backports WordPress/gutenberg#76266. Backports WordPress/gutenberg#76327. Props jorgefilipecosta, gziolo, swissspidy, flixos90. Fixes #64819. Built from https://develop.svn.wordpress.org/trunk@61985 git-svn-id: http://core.svn.wordpress.org/trunk@61267 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This updates the pinned hash from the `gutenberg` from `9b8144036fa5faf75de43d4502ff9809fcf689ad` to `8c78d87453509661a9f28f978ba2c242d515563b`. The following changes are included: - Navigation Editor: Allow any blocks to be inserted by gating contentOnly insertion rules to section blocks (WordPress/gutenberg#76189) - Add `fetchpriority=low` to `IMG` tags in collapsed Details blocks (WordPress/gutenberg#76269) - Connectors: Add logo URL support for custom AI providers (WordPress/gutenberg#76190) - Cover Block: Add a playlist parameter to loop YouTube background videos. (WordPress/gutenberg#76004) - Connectors: Memoize getConnectors selector (WordPress/gutenberg#76339) - HTML Block: Fix broken layout (WordPress/gutenberg#76278) - Tests: Skip connector logo URL tests when AI Client is unavailable (WordPress/gutenberg#76343) - Navigation Overlay: Explicitly set fetchpriority for images (WordPress/gutenberg#76208) - Connectors: Show API key source for env vars and wp-config constants (WordPress/gutenberg#76355) - Connectors: Move API key validation and masking to REST dispatch level (WordPress/gutenberg#76327) - Connectors: Replace apiFetch with core-data store selectors (WordPress/gutenberg#76333) - Do not sync local attributes (WordPress/gutenberg#76267) - Add `fetchpriority=low` to `IMG` tags in collapsed Accordion Item blocks (WordPress/gutenberg#76336) - Implement disconnection debounce after initial connection (WordPress/gutenberg#76114) - Allow Post Content to be edited when 'Show template' is active and Post content is nested in a Template Part (WordPress/gutenberg#76305) - Fix: Document Bar: Back button flickers (WordPress/gutenberg#76320) - RTC: Move event hooks from editor to core-data (WordPress/gutenberg#76358) - fix(navigation): prevent right-justified submenu overflow in custom overlays (WordPress/gutenberg#76360) - Connectors: Add connectors registry for extensibility (WordPress/gutenberg#76364) - Connectors: Add empty state when no connectors are registered (WordPress/gutenberg#76375) - Temp: Disable RTC in the site editor (WordPress/gutenberg#76223) - Connectors: Add AI Experiments plugin callout with install/activate functionality (WordPress/gutenberg#76379) - Editor: Polish real-time collaboration presence UI and move Avatar to editor package (WordPress/gutenberg#75652) (WordPress/gutenberg#76365) - RTC: Add collaborator selection highlighting in rich text (WordPress/gutenberg#76107) - Sync changes from `wp_enqueue_global_styles()` to Gutenberg override (WordPress/gutenberg#76127) - [RTC] Fix performance regression on post save (WordPress/gutenberg#76370) - Media: Enable AVIF support for client-side uploads (WordPress/gutenberg#76371) - Connectors: Move plugin status computation to script module data (WordPress/gutenberg#76409) - Revisions: Skip rendered fields in REST API responses (WordPress/gutenberg#76347) - E2E Tests: Add connector setup flow tests with test AI provider (WordPress/gutenberg#76433) - RTC: Place sync connection modal in front of popover (WordPress/gutenberg#76431) - Connectors: Sync PHP code with WordPress Core (WordPress/gutenberg#76443) - Editor: Show own presence in collaborative editing sessions (WordPress/gutenberg#76413) (WordPress/gutenberg#76445) A full list of changes can be found on GitHub: https://github.com/WordPress/gutenberg/compare/9b8144036fa5faf75de43d4502ff9809fcf689ad…8c78d87453509661a9f28f978ba2c242d515563b. Log created with: git log --reverse --format="- %s" 9b8144036fa5faf75de43d4502ff9809fcf689ad..8c78d87453509661a9f28f978ba2c242d515563b | sed 's|#\([0-9][0-9]*\)|https://github.com/WordPress/gutenberg/pull/\1|g; /github\.com\/WordPress\/gutenberg\/pull/!d' | pbcopy See #64595. git-svn-id: https://develop.svn.wordpress.org/trunk@61988 602fd350-edb4-49c9-b593-d223f7449a82
This updates the pinned hash from the `gutenberg` from `9b8144036fa5faf75de43d4502ff9809fcf689ad` to `8c78d87453509661a9f28f978ba2c242d515563b`. The following changes are included: - Navigation Editor: Allow any blocks to be inserted by gating contentOnly insertion rules to section blocks (WordPress/gutenberg#76189) - Add `fetchpriority=low` to `IMG` tags in collapsed Details blocks (WordPress/gutenberg#76269) - Connectors: Add logo URL support for custom AI providers (WordPress/gutenberg#76190) - Cover Block: Add a playlist parameter to loop YouTube background videos. (WordPress/gutenberg#76004) - Connectors: Memoize getConnectors selector (WordPress/gutenberg#76339) - HTML Block: Fix broken layout (WordPress/gutenberg#76278) - Tests: Skip connector logo URL tests when AI Client is unavailable (WordPress/gutenberg#76343) - Navigation Overlay: Explicitly set fetchpriority for images (WordPress/gutenberg#76208) - Connectors: Show API key source for env vars and wp-config constants (WordPress/gutenberg#76355) - Connectors: Move API key validation and masking to REST dispatch level (WordPress/gutenberg#76327) - Connectors: Replace apiFetch with core-data store selectors (WordPress/gutenberg#76333) - Do not sync local attributes (WordPress/gutenberg#76267) - Add `fetchpriority=low` to `IMG` tags in collapsed Accordion Item blocks (WordPress/gutenberg#76336) - Implement disconnection debounce after initial connection (WordPress/gutenberg#76114) - Allow Post Content to be edited when 'Show template' is active and Post content is nested in a Template Part (WordPress/gutenberg#76305) - Fix: Document Bar: Back button flickers (WordPress/gutenberg#76320) - RTC: Move event hooks from editor to core-data (WordPress/gutenberg#76358) - fix(navigation): prevent right-justified submenu overflow in custom overlays (WordPress/gutenberg#76360) - Connectors: Add connectors registry for extensibility (WordPress/gutenberg#76364) - Connectors: Add empty state when no connectors are registered (WordPress/gutenberg#76375) - Temp: Disable RTC in the site editor (WordPress/gutenberg#76223) - Connectors: Add AI Experiments plugin callout with install/activate functionality (WordPress/gutenberg#76379) - Editor: Polish real-time collaboration presence UI and move Avatar to editor package (WordPress/gutenberg#75652) (WordPress/gutenberg#76365) - RTC: Add collaborator selection highlighting in rich text (WordPress/gutenberg#76107) - Sync changes from `wp_enqueue_global_styles()` to Gutenberg override (WordPress/gutenberg#76127) - [RTC] Fix performance regression on post save (WordPress/gutenberg#76370) - Media: Enable AVIF support for client-side uploads (WordPress/gutenberg#76371) - Connectors: Move plugin status computation to script module data (WordPress/gutenberg#76409) - Revisions: Skip rendered fields in REST API responses (WordPress/gutenberg#76347) - E2E Tests: Add connector setup flow tests with test AI provider (WordPress/gutenberg#76433) - RTC: Place sync connection modal in front of popover (WordPress/gutenberg#76431) - Connectors: Sync PHP code with WordPress Core (WordPress/gutenberg#76443) - Editor: Show own presence in collaborative editing sessions (WordPress/gutenberg#76413) (WordPress/gutenberg#76445) A full list of changes can be found on GitHub: https://github.com/WordPress/gutenberg/compare/9b8144036fa5faf75de43d4502ff9809fcf689ad…8c78d87453509661a9f28f978ba2c242d515563b. Log created with: git log --reverse --format="- %s" 9b8144036fa5faf75de43d4502ff9809fcf689ad..8c78d87453509661a9f28f978ba2c242d515563b | sed 's|#\([0-9][0-9]*\)|https://github.com/WordPress/gutenberg/pull/\1|g; /github\.com\/WordPress\/gutenberg\/pull/!d' | pbcopy See #64595. Built from https://develop.svn.wordpress.org/trunk@61988 git-svn-id: http://core.svn.wordpress.org/trunk@61270 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Summary
sanitize_callbackto arest_post_dispatchfilter — keys are validated on POST and reverted to empty if invalidoption_{name}filter to the same dispatch filter — raw keys never appear in REST responses_gutenberg_get_real_api_keyhelper (no longer needed since masking doesn't happen at theget_optionlevel)_gutenberg_pass_default_connector_keys_to_ai_clientto useget_optiondirectlyTest plan
/wp/v2/settings?_fields=connectors_ai_openai_api_keyreturns masked valueget_option('connectors_ai_openai_api_key')returns the real (unmasked) value