Make WordPress Core

Opened 5 weeks ago

Closed 4 weeks ago

Last modified 4 weeks ago

#64861 closed defect (bug) (fixed)

_wp_connectors_init() should sanitize provider ID

Reported by: pers's profile PerS Owned by: gziolo's profile gziolo
Milestone: 7.0 Priority: normal
Severity: normal Version: trunk
Component: AI Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

_wp_connectors_init() should sanitize provider IDs

Summary

_wp_connectors_init() passes AI Client provider IDs directly to WP_Connector_Registry::register(), which rejects IDs containing hyphens. Third-party providers using hyphens in their ID (e.g. azure-openai) are silently dropped from the connector registry, causing downstream features (AI Experiments) to report missing credentials even though the provider works correctly at the AiClient level.

Steps to Reproduce

  1. Register an AI Client provider with a hyphenated ID:
    new ProviderMetadata(
        'my-custom-provider',  // contains a hyphen
        'My Custom Provider',
        ProviderTypeEnum::cloud(),
        'https://example.com/keys',
        RequestAuthenticationMethod::apiKey()
    );
    
  2. Activate the provider plugin.
  3. Go to Settings → Connectors — the provider does not appear.
  4. Activate AI Experiments — the "no valid AI Connector" warning is shown.
  5. Check debug log — a _doing_it_wrong notice from WP_Connector_Registry::register() is present:

    Connector ID must contain only lowercase alphanumeric characters and underscores.

Expected Behavior

Provider IDs with hyphens should either:

  1. Be automatically sanitized (hyphens → underscores) by _wp_connectors_init() before passing to the registry, or
  2. The registry should accept hyphens in connector IDs (relax the regex to /^[a-z0-9_-]+$/).

Option 1 is preferred because it maintains the existing connector ID format while being transparent to provider authors.

Proposed Patch

In wp-includes/connectors.php, inside _wp_connectors_init(), sanitize the connector ID before use:

foreach ( $ai_registry->getRegisteredProviderIds() as $connector_id ) {
    // Sanitize: WP_Connector_Registry only allows [a-z0-9_].
    $connector_id = str_replace( '-', '_', $connector_id );

    $provider_class_name = $ai_registry->getProviderClassName( $connector_id );
    // ... rest of the loop
}

Note: getProviderClassName() also needs to accept the original (unsanitized) ID since the AiClient registry stores providers under the original key. The sanitization should only apply to the connector registry side.

A more robust approach:

foreach ( $ai_registry->getRegisteredProviderIds() as $provider_id ) {
    // Connector IDs only allow [a-z0-9_]; provider IDs may use hyphens.
    $connector_id        = str_replace( '-', '_', $provider_id );
    $provider_class_name = $ai_registry->getProviderClassName( $provider_id );
    // ... use $connector_id for registry->register(), $provider_id for AiClient lookups
}

Impact

  • All three built-in providers (anthropic, google, openai) use single-word IDs and are unaffected.
  • Any third-party provider using a hyphenated ID is silently broken.
  • The _doing_it_wrong notice is only visible with WP_DEBUG enabled and is easy to miss.

Component

Connectors API / AI Client

Version

7.0-beta5

  • wp-includes/class-wp-connector-registry.php — ID validation regex
  • wp-includes/connectors.php_wp_connectors_init() function
  • AI Experiments plugin includes/helpers.phphas_ai_credentials() check

Change History (10)

This ticket was mentioned in PR #11260 on WordPress/wordpress-develop by @PerS.


5 weeks ago
#1

  • Keywords has-patch has-unit-tests added

## Patch for #64861: _wp_connectors_init() should sanitize provider IDs

### Problem

_wp_connectors_init() passes AI Client provider IDs directly to WP_Connector_Registry::register(), which validates IDs against /^[a-z0-9_]+$/. Third-party providers using hyphens in their ID (e.g. azure-openai) are silently rejected with a _doing_it_wrong notice, causing them to be missing from the connector registry even though they work correctly at the AiClient level.

### Approach

Option 1 from the ticket: sanitize hyphens → underscores before passing to the registry, while preserving the original provider ID for AiClient lookups.

### Changes

class-wp-connector-registry.php

  • register() now accepts and stores an optional provider_id field in connector data, allowing callers to preserve the original AI Client provider ID when it differs from the sanitized connector ID.
  • Updated @phpstan-type Connector and $args docblock accordingly.

connectors.php

  • In _wp_connectors_init(): the loop variable from getRegisteredProviderIds() is now $provider_id. A sanitized $connector_id = str_replace( '-', '_', $provider_id ) is used as the registry key. getProviderClassName() continues to receive the original $provider_id since the AiClient registry stores providers under their original key. Both existing defaults and new providers store 'provider_id' => $provider_id in the connector data.
  • Four downstream functions that iterate over registered connectors and call back into the AiClient API now extract $provider_id = $connector_data['provider_id'] ?? $connector_id and use it for all AiClient calls:
    • _wp_connectors_rest_settings_dispatch() — key validation via _wp_connectors_is_ai_api_key_valid()
    • _wp_register_default_connector_settings()hasProvider() check
    • _wp_connectors_pass_default_keys_to_ai_client()hasProvider(), _wp_connectors_get_api_key_source(), setProviderRequestAuthentication()
    • _wp_connectors_get_connector_script_module_data()hasProvider(), isProviderConfigured(), _wp_connectors_get_api_key_source()
  • Updated return-type docblocks for wp_get_connector() and wp_get_connectors() to document the optional provider_id field.

wpConnectorRegistry.php

Four new tests:

  • test_register_stores_provider_id — provider_id is preserved in returned connector data
  • test_register_omits_provider_id_when_not_provided — no provider_id key when absent from args
  • test_register_omits_provider_id_when_empty — no provider_id key when empty string
  • test_get_registered_includes_provider_id — provider_id round-trips through get_registered()

### Impact

  • The three built-in providers (anthropic, google, openai) have no hyphens and are unaffected.
  • Third-party providers with hyphenated IDs will now be registered under the underscore variant (e.g. azure-openaiazure_openai) and appear correctly in Settings → Connectors.
  • The provider_id field falls back to the connector ID via ??, so existing code consuming connector data is unaffected.

Trac ticket: https://core.trac.wordpress.org/ticket/64861

## Use of AI Tools

GitHub Copilot + Opus 4.6 used to review the patch

@JeffPaul commented on PR #11260:


5 weeks ago
#2

@felixarntz @JasonTheAdams @justlevine

#3 @gziolo
5 weeks ago

  • Milestone changed from Awaiting Review to 7.0

#4 @gziolo
5 weeks ago

  • Type changed from enhancement to defect (bug)

This ticket was mentioned in PR #11285 on WordPress/wordpress-develop by @gziolo.


5 weeks ago
#5

Tract ticket: https://core.trac.wordpress.org/ticket/64861## Summary

  • Changes connector ID validation from /^[a-z0-9_]+$/ to /^[a-z0-9-]+$/, aligning with the naming conventions used by the Abilities API, and WordPress.org plugin slugs (all kebab-case, no underscores).
  • Hyphens in connector IDs are normalized to underscores when auto-generating setting_name (e.g. azure-openaiconnectors_ai_azure_openai_api_key), consistent with how the WP AI Client library already builds env var names.
  • Updates existing tests to use hyphen-based IDs and adds three new tests covering the new behavior.

Fixes #64861.

## Test plan

  • [ ] Run npm run test:php -- --filter=Tests_Connectors_WpConnectorRegistry — all 31 tests pass.
  • [ ] Register a third-party AI provider with a hyphenated ID (e.g. azure-openai) and confirm it appears on Settings → Connectors without a _doing_it_wrong notice.
  • [ ] Confirm the generated option name is connectors_ai_azure_openai_api_key (underscores).

🤖 Generated with Claude Code

@gziolo commented on PR #11260:


5 weeks ago
#6

I proposed an alternate approach in https://github.com/WordPress/wordpress-develop/pull/11285. Instead of tracking the orginal provider name, we can align with how names/slugs are structured in WordPress (see ability name, or plugin slugs) and allow - instead of _. It's still an open question whether we should include similar validation for AI providers, because it allows basically everything today, with the remark that when handling env variables and PHP const names, - gets replaced with _, so it could be worth exploring some stricter standardization.

@gziolo commented on PR #11285:


5 weeks ago
#7

It's still an open question whether we should include similar validation for AI providers, because it currently allows almost anything, with the note that when handling environment variables and PHP constant names, - gets replaced with _. Therefore, it might be worth exploring a stricter standardization in the PHP API client to avoid mismatches, too.

#8 @gziolo
5 weeks ago

  • Owner set to gziolo
  • Status changed from new to assigned

#9 @gziolo
4 weeks ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 62056:

Connectors: Allow hyphens in connector IDs

Expands the connector ID validation regex from /^[a-z0-9_]+$/ to /^[a-z0-9_-]+$/, aligning with the PHP AI Client library naming conventions. Hyphens are normalized to underscores when generating setting_name (e.g., azure-openaiconnectors_ai_azure_openai_api_key).

Developed in https://github.com/WordPress/wordpress-develop/pull/11285.

Props pers, gziolo, jorgefilipecosta, westonruter, flixos90, mukesh27.
Fixes #64861.

This ticket was mentioned in Slack in #core-ai by soderlind. View the logs.


4 weeks ago

Note: See TracTickets for help on using tickets.