Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: added

Dashboard widgets: carry declarative actions and icon references from the widget manifest to the registry and the widget-modules REST record.
22 changes: 22 additions & 0 deletions projects/packages/premium-analytics/src/class-widget-type.php
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,28 @@ class Widget_Type {
*/
public $help = null;

/**
* Registered icon name (`collection/icon-name`), resolved on the client
* through the application's icon resolver.
*
* Null when the widget did not declare the field.
*
* @var string|null
*/
public $icon = null;

/**
* Declarative actions the widget exposes. Each entry carries `id`,
* `label`, `href`, and optional `download`/`openInNewTab`/`icon`/
* `relevance`. Labels are translated at registration time using the
* widget's text domain.
*
* Null when the widget did not declare the field.
*
* @var array|null
*/
public $actions = null;

/**
* Alternative terms used to match the widget type when searching,
* e.g. "calendar" for an events widget. Translated at registration
Expand Down
1 change: 1 addition & 0 deletions projects/packages/premium-analytics/src/widget-i18n.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@
"content": "widget help content",
"links": [ { "label": "widget help link label" } ]
},
"actions": [ { "label": "widget action label" } ],
"keywords": [ "widget keyword" ]
}
2 changes: 2 additions & 0 deletions projects/packages/premium-analytics/src/widget-modules.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ function get_widget_modules_response() {
'title' => $widget_type->title,
'description' => $widget_type->description,
'help' => $widget_type->help,
'icon' => $widget_type->icon,
'actions' => $widget_type->actions,
'keywords' => $widget_type->keywords,
);
}
Expand Down
158 changes: 152 additions & 6 deletions projects/packages/premium-analytics/src/widget-types.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
* Copies the wp-build manifest (`jpa_get_registered_widget_modules()`) into the
* in-memory Widget_Type_Registry, so the plugin queries the registry instead
* of re-parsing the manifest. On the way in, user-facing metadata strings are
* translated (per the widget-i18n.json schema) and the `help` note sanitized.
* translated (per the widget-i18n.json schema) and the `help`, `icon`, and
* `actions` fields sanitized.
*
* This is the problem-agnostic "core" layer (a PA-namespaced copy of the
* experimental Gutenberg API): it exposes the hooks a consumer uses to scope
Expand Down Expand Up @@ -58,10 +59,11 @@ function get_widget_metadata_i18n_schema() {
/**
* Translates a widget's user-facing metadata strings.
*
* Runs `title`, `description`, `help`, and `keywords` through the widget
* i18n schema, leaving every other key untouched. Unlike the upstream copy,
* a widget with no `textdomain` falls back to the package text domain
* instead of skipping translation: every bundled widget shares it.
* Runs `title`, `description`, `help`, `actions`, and `keywords` through
* the widget i18n schema, leaving every other key untouched. Unlike the
* upstream copy, a widget with no `textdomain` falls back to the package
* text domain instead of skipping translation: every bundled widget
* shares it.
*
* @param array $widget Widget data from the build manifest.
* @return array Widget data with its translatable strings localized.
Expand All @@ -70,7 +72,7 @@ function translate_widget_metadata( $widget ) {
$textdomain = ! empty( $widget['textdomain'] ) ? $widget['textdomain'] : 'jetpack-premium-analytics-pkg';
$i18n_schema = get_widget_metadata_i18n_schema();

foreach ( array( 'title', 'description', 'help', 'keywords' ) as $field ) {
foreach ( array( 'title', 'description', 'help', 'actions', 'keywords' ) as $field ) {
if ( isset( $widget[ $field ] ) && isset( $i18n_schema->$field ) ) {
$widget[ $field ] = translate_settings_using_i18n_schema( $i18n_schema->$field, $widget[ $field ], $textdomain );
}
Expand Down Expand Up @@ -125,6 +127,148 @@ function sanitize_widget_help( $help ) {
return $sanitized;
}

/**
* Resolves an action href to the form `esc_url_raw()` can judge.
*
* Absolute, scheme-relative, root-relative, and single-segment admin `.php`
* hrefs pass through unchanged. Returns '' for path traversal and for any
* other relative href, so `esc_url_raw()` cannot invent `http://filename`.
* Unlike upstream, widget-local files are never resolved: `widgets/` does
* not ship with the package, only its build output does.
*
* @param string $href Action href.
* @return string The href to escape, or ''.
*/
function resolve_widget_action_href( $href ) {
if ( ! is_string( $href ) || '' === $href ) {
return '';
}

// Absolute, scheme-relative, or schemed, including URLs with `..` in the path.
if ( preg_match( '#^([a-z][a-z0-9+.-]*:)?//#i', $href ) || str_contains( $href, ':' ) ) {
return $href;
}

// Root-relative paths (e.g. /wp-admin/..., /report.csv).
if ( str_starts_with( $href, '/' ) ) {
return $href;
}

if ( str_contains( $href, '..' ) ) {
return '';
}

$path_only = preg_split( '/[?#]/', $href, 2 )[0];
if ( str_ends_with( strtolower( $path_only ), '.php' ) ) {
// Single-segment admin entry points stay as-is. Deeper relative
// paths would come out of `esc_url_raw()` as `http://` URLs.
return str_contains( $path_only, '/' ) ? '' : $href;
Comment on lines +157 to +165

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of things I hit poking at this on my site, both non-blocking since nothing declares actions yet: export_report.php?type=csv comes back as http://export_report.php?type=csv (esc_url_raw() only exempts [a-z0-9-]+.php, this check lets anything .php through), and any : in the query of a relative admin href gets read as a scheme by kses and the action is dropped with the notice - e.g. admin.php?page=jetpack-premium-analytics-wp-admin&p=/reports/posts?from=2026-08-05T00:00:00.000+00:00, while the %3A encoded form survives. Same story for .. in the query (range=2024-01-01..2024-01-31) tripping the traversal check, since it runs on $href rather than $path_only.

I think anchoring the single-segment entry points through admin_url() and moving the .. check onto $path_only would cover all three, at the cost of the record carrying absolute admin URLs (and the tests wanting the absolute form). Up to you whether it's worth doing here or in the follow-up that adds the first real action.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, all three reproduce as described. Nothing declares an action yet and the fix changes the href shape the links capability will match, so I filed it as a follow-up with your admin_url() + $path_only proposal, plus the upstream report since the resolver is identical there. The Widget_Modules_Test fatal and the WidgetActionRecord reuse are in the two latest commits.

}

return '';
}

/**
* Sanitizes widget actions to `id` / `label` / `href` (via `esc_url_raw()`),
* plus optional `download` / `openInNewTab` / `icon` / `relevance`. Drops
* incomplete or unsafe entries and reports dropped hrefs through
* `_doing_it_wrong()`; a malformed `icon` or `relevance` drops the key, never
* the action, and a `download` name that sanitizes to nothing becomes `true`.
*
* @param mixed $actions Actions from the build manifest.
* @return array|null Sanitized actions, or null when none survive.
*/
function sanitize_widget_actions( $actions ) {
if ( ! is_array( $actions ) ) {
return null;
}

$sanitized = array();
foreach ( $actions as $action ) {
if (
! is_array( $action ) ||
! isset( $action['id'] ) ||
! isset( $action['label'] ) ||
! isset( $action['href'] ) ||
! is_string( $action['id'] ) ||
! is_string( $action['label'] ) ||
! is_string( $action['href'] ) ||
'' === $action['id'] ||
'' === $action['label'] ||
'' === $action['href']
) {
continue;
}

$href = esc_url_raw( resolve_widget_action_href( $action['href'] ) );
if ( ! $href ) {
$message = sprintf(
/* translators: 1: Widget action id. 2: Declared action href. */
__( 'Dropped widget action "%1$s": href "%2$s" is not an allowed URL.', 'jetpack-premium-analytics-pkg' ),
$action['id'],
$action['href']
);
// One line: tools/replace-next-version-tag.sh only rewrites the token in a single-line call.
_doing_it_wrong( __FUNCTION__, esc_html( $message ), 'jetpack-premium-analytics-$$next-version$$' );
continue;
}

$entry = array(
'id' => $action['id'],
'label' => $action['label'],
'href' => $href,
);

if ( isset( $action['download'] ) ) {
if ( is_bool( $action['download'] ) ) {
$entry['download'] = $action['download'];
} else {
// A name that sanitizes to nothing keeps the download under the original name; only `false` navigates.
$filename = sanitize_file_name( (string) $action['download'] );
$entry['download'] = '' !== $filename ? $filename : true;
}
}

if ( isset( $action['openInNewTab'] ) ) {
$entry['openInNewTab'] = (bool) $action['openInNewTab'];
}

if ( isset( $action['icon'] ) ) {
$icon = sanitize_widget_icon( $action['icon'] );
if ( $icon ) {
$entry['icon'] = $icon;
}
}

if ( isset( $action['relevance'] ) && in_array( $action['relevance'], array( 'high', 'medium', 'low' ), true ) ) {
$entry['relevance'] = $action['relevance'];
}

$sanitized[] = $entry;
}

return $sanitized ? $sanitized : null;
}

/**
* Constrains a widget icon reference to a registered icon name
* (`collection/icon-name`); anything else drops silently to no icon.
*
* @param mixed $icon Icon reference from the build manifest.
* @return string|null The icon name, or null when the shape does not match.
*/
function sanitize_widget_icon( $icon ) {
if ( ! is_string( $icon ) || '' === $icon ) {
return null;
}

if ( ! preg_match( '#^[a-z0-9](?:[a-z0-9_-]*[a-z0-9])?/[a-z0-9](?:[a-z0-9_-]*[a-z0-9])?$#', $icon ) ) {
return null;
}

return $icon;
}

/**
* Hydrates the widget type registry from the build manifest.
*
Expand Down Expand Up @@ -173,6 +317,8 @@ function register_widget_types() {
'title' => $widget['title'] ?? null,
'description' => $widget['description'] ?? null,
'help' => sanitize_widget_help( $widget['help'] ?? null ),
'icon' => sanitize_widget_icon( $widget['icon'] ?? null ),
'actions' => sanitize_widget_actions( $widget['actions'] ?? null ),
'keywords' => $widget['keywords'] ?? null,
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
use PHPUnit\Framework\Attributes\RunInSeparateProcess;
use PHPUnit\Framework\TestCase;

require_once __DIR__ . '/traits/trait-widget-manifest-fixture.php';

/**
* Tests for the Analytics class. Also covers ensure_widget_registry_ready(): only
* test_rest_request_still_serves_the_widget_manifest exercises it, and php-code-coverage
Expand All @@ -26,6 +28,7 @@
#[CoversClass( Analytics::class )]
#[CoversFunction( 'Automattic\\Jetpack\\PremiumAnalytics\\ensure_widget_registry_ready' )]
class Analytics_Test extends TestCase {
use Widget_Manifest_Fixture_Trait;

const MENU_SLUG = 'jetpack-premium-analytics-wp-admin';
const MENU_HOOKNAME = 'toplevel_page_' . self::MENU_SLUG;
Expand Down Expand Up @@ -430,24 +433,6 @@ public function test_rest_request_still_serves_the_widget_manifest() {
}
}

/**
* Point the manifest require at the fixture manifest.
*
* @return string
*/
public function use_fixture_widget_manifest() {
return __DIR__ . '/fixtures/build-entry/widgets.php';
}

/**
* Point the manifest at a missing file.
*
* @return string
*/
public function use_absent_widget_manifest() {
return __DIR__ . '/fixtures/build-entry/no-such-widgets.php';
}

/**
* The wp-admin integrated dashboard slug in admin is recognized as a dashboard request.
*/
Expand Down
Loading
Loading