PluginProbe
ThinkRank AI SEO – AI SEO Plugin for WordPress: Schema, XML Sitemaps, Meta Tags, Search Console & Local SEO / 2.7.0
ThinkRank AI SEO – AI SEO Plugin for WordPress: Schema, XML Sitemaps, Meta Tags, Search Console & Local SEO v2.7.0
2.7.0 2.6.0 2.5.0 2.4.0 2.3.0 2.2.0 2.1.1 2.1.0 2.0.2 2.0.1 2.0.0 1.32.0 1.31.0 1.30.0 1.29.0 1.28.0 1.27.0 1.26.0 1.25.0 trunk 1.0.0 1.0.1 1.0.2 1.1.0 1.10.0 All 48 releases
← All changes | includes/api/class-settings-management-endpoint.php +366 -27 1.29.02.7.0 View file →
@@ -92,10 +92,15 @@
92 92 'social_media' => 'Social Media & Open Graph',
93 93 'sitemap' => 'XML Sitemap Management',
94 94 'integrations' => 'External Integrations',
95 95 'analytics_integration' => 'Analytics Integration',
96 - 'seo_analytics' => 'SEO Analytics & Intelligence',
97 - 'global_defaults' => 'Global Default Settings'
96 + 'seo_analytics' => 'SEO Analytics & Intelligence'
97 + // 'global_defaults' was listed here but is registered in no settings
98 + // store and read by no client — the only mention in the codebase was
99 + // this label. Every save against it reached the compound write with
100 + // nothing to persist to and answered 500, so accepting the name only
101 + // promised a category that could never be stored. It now falls through
102 + // to the 400 invalid_category branch like any other unknown name (#371).
98 103 ];
99 104
100 105 /**
101 106 * Constructor
@@ -116,9 +121,14 @@
116 121 */
117 122 private array $seo_manager_classes = [
118 123 'site_identity' => Site_Identity_Manager::class,
119 124 'performance_monitoring' => Performance_Monitoring_Manager::class,
120 - 'ai_content_analyzer' => AI_Content_Analyzer::class,
125 + // Keyed by the endpoint's own category name. It was 'ai_content_analyzer',
126 + // which appears in no other registry, so the route rejected it with 400
127 + // invalid_category and this manager was never reachable — while the
128 + // endpoint's actual category, 'content_analysis', had no manager and
129 + // therefore nowhere to persist (#371).
130 + 'content_analysis' => AI_Content_Analyzer::class,
121 131 'content_optimization' => Content_Optimization_Manager::class,
122 132 'schema_management' => Schema_Management_System::class,
123 133 'social_media' => Social_Meta_Manager::class,
124 134 'sitemap' => Sitemap_Generator::class,
@@ -149,8 +159,39 @@
149 159 return $this->seo_managers[$category];
150 160 }
151 161
152 162 /**
163 + * Read a category from whichever store actually owns it.
164 + *
165 + * The generic store returns `[]` for the eight SEO categories: it looks for
166 + * rows whose key carries a `<category>_` prefix, and the rows carry no such
167 + * prefix — `social_media` is stored as `social_meta`, `schema_management` as
168 + * `schema_management_system`, and their keys are bare (`og_site_name`). So a
169 + * direct `Settings_Manager::get_settings()` reports a configured site as
170 + * having no settings at all.
171 + *
172 + * Writes never had the problem, because the write path already falls back to
173 + * the owning manager. That asymmetry is what made this invisible from the UI
174 + * and dangerous underneath it: the pre-reset rollback snapshotted `[]` and
175 + * then defaults were written over live settings, so Reset could not be undone
176 + * (#689). Every read goes through here now, so there is one place to be wrong.
177 + *
178 + * @since 2.7.0
179 + *
180 + * @param string $category Category key.
181 + * @param string $context_type Optional. Context type. Default 'site'.
182 + * @param int|null $context_id Optional. Context ID.
183 + * @return array The category's stored settings.
184 + */
185 + private function read_category(string $category, string $context_type = 'site', ?int $context_id = null): array {
186 + if ($this->has_seo_manager($category)) {
187 + return (array) $this->get_seo_manager($category)->get_settings($context_type, $context_id);
188 + }
189 +
190 + return (array) $this->settings_manager->get_settings($category, $context_type, $context_id);
191 + }
192 +
193 + /**
153 194 * Register API routes
154 195 *
155 196 * @since 1.0.0
156 197 */
@@ -405,8 +446,56 @@
405 446 *
406 447 * @param array $settings Flat key => value map from the request.
407 448 * @return array Map with masked secret values removed.
408 449 */
450 + /**
451 + * Drop setting keys the category does not define.
452 + *
453 + * The known set is whatever describes the category: the generic store's key
454 + * list, and the dedicated manager's default settings when one owns it.
455 + * Fails open — if neither store can describe the category there is nothing
456 + * to check against, and silently dropping everything would be worse than
457 + * storing an unknown key.
458 + *
459 + * @since 2.0.1
460 + *
461 + * @param array $settings Incoming settings.
462 + * @param string $category Settings category.
463 + * @param string $context_type Context the write is scoped to.
464 + * @return array Settings limited to recognised keys.
465 + */
466 + private function filter_known_setting_keys(array $settings, string $category, string $context_type): array {
467 + $known = [];
468 +
469 + // $this->setting_categories maps category => label; the key lists live
470 + // in the generic store.
471 + $known = array_merge($known, $this->settings_manager->get_category_keys($category));
472 +
473 + if ($this->has_seo_manager($category)) {
474 + $known = array_merge(
475 + $known,
476 + array_keys($this->get_seo_manager($category)->get_default_settings($context_type))
477 + );
478 + }
479 +
480 + /**
481 + * Filter the setting keys a category accepts.
482 + *
483 + * @since 2.0.1
484 + *
485 + * @param string[] $known Recognised setting keys.
486 + * @param string $category Settings category.
487 + * @param string $context_type Context the write is scoped to.
488 + */
489 + $known = apply_filters('thinkrank_known_setting_keys', $known, $category, $context_type);
490 +
491 + if (empty($known)) {
492 + return $settings;
493 + }
494 +
495 + return array_intersect_key($settings, array_flip($known));
496 + }
497 +
409 498 private function strip_masked_secrets(array $settings): array {
410 499 foreach ($settings as $key => $value) {
411 500 if (!in_array($key, self::SENSITIVE_SETTING_KEYS, true)) {
412 501 continue;
@@ -438,10 +527,10 @@
438 527 if (!isset($this->setting_categories[$category])) {
439 528 continue;
440 529 }
441 530
442 - // Get settings for each category using Settings Manager
443 - $category_settings = $this->settings_manager->get_settings($category);
531 + // Get settings for each category from the store that owns it.
532 + $category_settings = $this->read_category($category);
444 533 $global_settings[$category] = $category_settings;
445 534
446 535 // Get schema if requested
447 536 if ($include_schema && $this->has_seo_manager($category)) {
@@ -577,9 +666,9 @@
577 666 // Get updated settings
578 667 $updated_settings = [];
579 668 foreach (array_keys($settings) as $category) {
580 669 if (isset($this->setting_categories[$category])) {
581 - $updated_settings[$category] = $this->settings_manager->get_settings($category);
670 + $updated_settings[$category] = $this->read_category($category);
582 671 }
583 672 }
584 673
585 674 return new WP_REST_Response([
@@ -624,9 +713,9 @@
624 713 );
625 714 }
626 715
627 716 // Get category settings
628 - $category_settings = $this->settings_manager->get_settings($category);
717 + $category_settings = $this->read_category($category);
629 718
630 719 // Get schema if requested
631 720 $schema = [];
632 721 if ($include_schema && $this->has_seo_manager($category)) {
@@ -701,11 +790,41 @@
701 790 ['status' => 400]
702 791 );
703 792 }
704 793
794 + // SECURITY: this route also accepts an object context and forwards it
795 + // to the category's SEO manager, which upserts rows keyed by that ID.
796 + // The `thinkrank_settings` capability authorises entry to the Settings
797 + // section — it is not authorisation to edit every post on the site — so
798 + // resolve and authorise the object before ANY write happens below (#367).
799 + $context_type = $request->get_param('context_type') ?? 'site';
800 + $context_id = $request->get_param('context_id');
801 + $context_id = null === $context_id ? null : (int) $context_id;
802 +
803 + $context_error = $this->authorize_settings_context($context_type, $context_id);
804 + if (is_wp_error($context_error)) {
805 + return $context_error;
806 + }
807 +
705 808 // Reads mask secrets; never persist a mask back over the real one.
706 809 $settings = $this->strip_masked_secrets($settings);
707 810
811 + // Drop keys the category does not define. This route persisted any
812 + // key it was handed — a probe key written through it is still
813 + // readable in the settings table afterwards — which bloats the
814 + // store and lets a client invent settings the plugin will never
815 + // read (#395). Mirrors the same guard on the schema and
816 + // social-media routes.
817 + $settings = $this->filter_known_setting_keys($settings, $category, $context_type);
818 +
819 + if (empty($settings)) {
820 + return new WP_Error(
821 + 'invalid_settings',
822 + "No recognized settings were provided for category: {$category}",
823 + ['status' => 400]
824 + );
825 + }
826 +
708 827 $validation_result = ['valid' => true];
709 828
710 829 // Validate settings if requested
711 830 if ($validate_before_update && $this->has_seo_manager($category)) {
@@ -723,24 +842,93 @@
723 842 );
724 843 }
725 844 }
726 845
727 - // Update settings
728 - $update_success = $this->settings_manager->update_settings($settings, $category);
846 + // Update settings. The context must be forwarded: update_settings()
847 + // defaults to the 'site' context, so a post-scoped request was also
848 + // silently rewriting the site-wide defaults (#367).
849 + $generic_update = $this->settings_manager->update_settings($settings, $category, $context_type, $context_id);
850 + $manager_update = null;
729 851
730 - // Also update through specific SEO manager if available
852 + // Also update through specific SEO manager if available. The context was
853 + // resolved and authorised above.
731 854 if ($this->has_seo_manager($category)) {
732 - $context_type = $request->get_param('context_type') ?? 'site';
733 - $context_id = $request->get_param('context_id') ?? null;
734 855 $manager_update = $this->get_seo_manager($category)->save_settings($context_type, $context_id, $settings);
735 - $update_success = $update_success && $manager_update;
736 856 }
737 857
858 + // null from a store means "this category is not mine", not "the write
859 + // failed" — the two registries use different category vocabularies, so
860 + // most categories are owned by exactly one store (#371). Judge only the
861 + // stores that actually attempted a write: the save succeeded if at least
862 + // one store owned the category and none of the owners failed. ANDing the
863 + // raw values reported 500 for every category the generic store does not
864 + // know, while the dedicated manager's row had already committed.
865 + $attempted = array_filter(
866 + [$generic_update, $manager_update],
867 + static fn($result) => null !== $result
868 + );
869 +
870 + $update_success = [] !== $attempted && !in_array(false, $attempted, true);
871 +
738 872 if (!$update_success) {
873 + // Name the settings that did not persist. The write is not
874 + // transactional, so "failed" can mean some keys saved and others
875 + // did not — without the list the UI can only show a generic
876 + // error and the user has no idea what to re-enter (#300).
877 + $failed_keys = $this->settings_manager->get_last_failed_keys();
878 +
879 + // Report which store failed. Collapsing both writes into one boolean
880 + // meant a committed manager row could be reported as a total failure,
881 + // hiding a persisted change behind a 500 (#367). Only a literal false
882 + // is a failure — null means the store does not own this category and
883 + // never attempted a write, so it must not be named here (#371).
884 + $stores_failed = [];
885 + if (false === $generic_update) {
886 + $stores_failed[] = 'settings';
887 + }
888 + if (false === $manager_update) {
889 + $stores_failed[] = 'category_manager';
890 + }
891 +
892 + // No store owns the category. That is a routing defect rather than a
893 + // failed write, and it is worth distinguishing: the settings were
894 + // never persisted anywhere, so reporting it as a plain write failure
895 + // would send the user back to re-enter values that have nowhere to go.
896 + if ([] === $attempted) {
897 + return new WP_Error(
898 + 'category_not_persistable',
899 + sprintf(
900 + 'No settings store is registered for category %s, so nothing was saved.',
901 + $category
902 + ),
903 + [
904 + 'status' => 500,
905 + 'failed_keys' => $failed_keys,
906 + 'stores_failed' => $stores_failed,
907 + 'partial_write' => false,
908 + ]
909 + );
910 + }
911 +
739 912 return new WP_Error(
740 913 'update_failed',
741 - "Failed to update settings for category: {$category}",
742 - ['status' => 500]
914 + empty($failed_keys)
915 + ? "Failed to update settings for category: {$category}"
916 + : sprintf(
917 + 'Failed to save %s in category %s. Other settings in this request were saved.',
918 + implode(', ', $failed_keys),
919 + $category
920 + ),
921 + [
922 + 'status' => 500,
923 + 'failed_keys' => $failed_keys,
924 + 'stores_failed' => $stores_failed,
925 + // True when more than one store attempted the write and they
926 + // disagreed, so the client knows the request was not a clean
927 + // no-op. Stores that did not own the category are excluded.
928 + 'partial_write' => in_array(true, $attempted, true)
929 + && in_array(false, $attempted, true),
930 + ]
743 931 );
744 932 }
745 933
746 934 // Clear analytics cache when GSC/GA settings change so fresh data is fetched
@@ -755,10 +943,20 @@
755 943
756 944 // Update category metadata
757 945 $this->update_category_metadata($category);
758 946
759 - // Get updated settings
760 - $updated_settings = $this->settings_manager->get_settings($category);
947 + // Get updated settings. Read them back from whichever store actually
948 + // owns the category: the generic store returns [] for the categories it
949 + // does not know, which would report a successful save as zero settings
950 + // and hand the UI an empty form to render (#371).
951 + //
952 + // Which store *accepted the write* is the wrong question to ask here,
953 + // and `sitemap` is the case that proves it: the generic store claims
954 + // that write (update_settings() returns true, not null) and then reads
955 + // the category back as [], so keying off $generic_update sent the one
956 + // read path that had been fixed straight back into the empty store.
957 + // Ownership is a property of the category, not of the last write (#689).
958 + $updated_settings = $this->read_category($category, $context_type, $context_id);
761 959
762 960 return new WP_REST_Response([
763 961 'success' => true,
764 962 'data' => [
@@ -935,9 +1133,9 @@
935 1133 if (!isset($this->setting_categories[$category])) {
936 1134 continue;
937 1135 }
938 1136
939 - $export_data[$category] = $this->settings_manager->get_settings($category);
1137 + $export_data[$category] = $this->read_category($category);
940 1138 }
941 1139
942 1140 // Never let secrets (API keys, OAuth tokens) leave the site in an
943 1141 // export file — strip them entirely.
@@ -1073,9 +1271,9 @@
1073 1271 }
1074 1272
1075 1273 try {
1076 1274 // Check if settings exist and handle overwrite
1077 - $existing_settings = $this->settings_manager->get_settings($category);
1275 + $existing_settings = $this->read_category($category);
1078 1276
1079 1277 if (!empty($existing_settings) && !$overwrite_existing) {
1080 1278 $import_results[$category] = [
1081 1279 'success' => false,
@@ -1148,9 +1346,9 @@
1148 1346 // Create backup data
1149 1347 $backup_data = [];
1150 1348 foreach ($categories as $category) {
1151 1349 if (isset($this->setting_categories[$category])) {
1152 - $backup_data[$category] = $this->settings_manager->get_settings($category);
1350 + $backup_data[$category] = $this->read_category($category);
1153 1351 }
1154 1352 }
1155 1353
1156 1354 // Create backup metadata
@@ -1401,9 +1599,9 @@
1401 1599 *
1402 1600 * @param WP_REST_Request $request Request object
1403 1601 * @return WP_REST_Response|WP_Error Response object
1404 1602 */
1405 - public function add_performance_indexes(WP_REST_Request $request): WP_REST_Response|WP_Error {
1603 + public function add_performance_indexes(WP_REST_Request $request) {
1406 1604 try {
1407 1605 // Import the Database_Schema class
1408 1606 if (!class_exists('ThinkRank\\Database\\Database_Schema')) {
1409 1607 require_once THINKRANK_PLUGIN_DIR . 'includes/database/class-database-schema.php';
@@ -1451,15 +1649,49 @@
1451 1649 * @since 1.0.0
1452 1650 *
1453 1651 * @return bool Permission status
1454 1652 */
1455 - public function check_read_permissions(): bool {
1653 + public function check_read_permissions(WP_REST_Request $request): bool {
1456 1654 // Plugin SEO/AI config is not subscriber-visible — require the same
1457 - // management capability as the write routes.
1458 - return \ThinkRank\Core\Capability_Manager::current_user_can('thinkrank_settings');
1655 + // management capability as the write routes, resolved per category so a
1656 + // role granted one section can reach that section and no other (#573).
1657 + return \ThinkRank\Core\Capability_Manager::current_user_can(
1658 + $this->capability_for_request($request)
1659 + );
1459 1660 }
1460 1661
1461 1662 /**
1663 + * The capability a settings-management request requires.
1664 + *
1665 + * Category routes belong to the section owning the category; every other
1666 + * route on this controller is plugin-wide configuration and stays on
1667 + * `thinkrank_settings`. The gate in Role_Manager::gate_rest() reaches the
1668 + * same answer through Capability_Manager::capability_for_route() — both are
1669 + * kept so neither layer alone is load-bearing.
1670 + *
1671 + * @since 2.1.3
1672 + *
1673 + * @param WP_REST_Request $request Request.
1674 + * @return string
1675 + */
1676 + private function capability_for_request(WP_REST_Request $request): string {
1677 + // URL params only. get_param() searches the JSON body, the POST body
1678 + // and the query string ahead of the route path, so on the routes that
1679 + // declare no {category} — /global, /validate, /schema, /export,
1680 + // /backup, /restore — it read pure caller input and let a request
1681 + // nominate the capability it would be checked against (#582). Reading
1682 + // the path is also what Role_Manager::gate_rest() does, so the two
1683 + // layers now agree and the claim above is true again.
1684 + $category = $request->get_url_params()['category'] ?? null;
1685 +
1686 + if (!is_string($category) || '' === $category) {
1687 + return 'thinkrank_settings';
1688 + }
1689 +
1690 + return \ThinkRank\Core\Capability_Manager::capability_for_settings_category($category);
1691 + }
1692 +
1693 + /**
1462 1694 * Check permissions for managing settings
1463 1695 *
1464 1696 * @since 1.0.0
1465 1697 *
@@ -1464,10 +1696,12 @@
1464 1696 * @since 1.0.0
1465 1697 *
1466 1698 * @return bool Permission status
1467 1699 */
1468 - public function check_manage_permissions(): bool {
1469 - return \ThinkRank\Core\Capability_Manager::current_user_can('thinkrank_settings');
1700 + public function check_manage_permissions(WP_REST_Request $request): bool {
1701 + return \ThinkRank\Core\Capability_Manager::current_user_can(
1702 + $this->capability_for_request($request)
1703 + );
1470 1704 }
1471 1705
1472 1706 /**
1473 1707 * Check permissions for administrator-only settings operations.
@@ -1746,13 +1980,100 @@
1746 1980 'required' => false,
1747 1981 'type' => 'boolean',
1748 1982 'default' => true,
1749 1983 'description' => 'Whether to validate settings before updating'
1984 + ],
1985 + // Declared so the REST schema validates/normalises them. They were read
1986 + // by the handler while undeclared, which skipped validation entirely (#367).
1987 + 'context_type' => [
1988 + 'required' => false,
1989 + 'type' => 'string',
1990 + 'enum' => ['site', 'post', 'page', 'product'],
1991 + 'default' => 'site',
1992 + 'description' => 'Object context these settings apply to'
1993 + ],
1994 + 'context_id' => [
1995 + 'required' => false,
1996 + 'type' => 'integer',
1997 + 'minimum' => 1,
1998 + 'description' => 'Object ID when context_type is not "site"'
1750 1999 ]
1751 2000 ];
1752 2001 }
1753 2002
1754 2003 /**
2004 + * Authorise the object context a category settings write targets.
2005 + *
2006 + * The Settings section capability is delegatable, so a non-administrator can
2007 + * reach this controller. Writing settings for a specific post is an edit of
2008 + * that post and must be authorised as one — mirroring the per-object check the
2009 + * social-media write route performs (#277, #367).
2010 + *
2011 + * @since 1.32.0
2012 + *
2013 + * @param string $context_type Requested context type.
2014 + * @param int|null $context_id Requested object ID.
2015 + * @return true|WP_Error True when the write is allowed, WP_Error otherwise.
2016 + */
2017 + private function authorize_settings_context(string $context_type, ?int $context_id) {
2018 + if ('site' === $context_type) {
2019 + return true;
2020 + }
2021 +
2022 + if (!in_array($context_type, ['post', 'page', 'product'], true)) {
2023 + return new WP_Error(
2024 + 'invalid_context',
2025 + 'Invalid context type provided',
2026 + ['status' => 400]
2027 + );
2028 + }
2029 +
2030 + if (!$context_id || $context_id <= 0) {
2031 + return new WP_Error(
2032 + 'invalid_context',
2033 + 'A valid context_id is required for non-site contexts',
2034 + ['status' => 400]
2035 + );
2036 + }
2037 +
2038 + $post = get_post($context_id);
2039 +
2040 + if (!$post || 'revision' === $post->post_type) {
2041 + return new WP_Error(
2042 + 'invalid_context',
2043 + 'The requested content could not be found',
2044 + ['status' => 404]
2045 + );
2046 + }
2047 +
2048 + // The declared context must match the one the front-end read path derives
2049 + // from the real post type, otherwise `page`/`product` can alias an arbitrary
2050 + // object and the row is written where nothing will ever read it. Mirrors
2051 + // Seo_Manager::get_context_type() — custom post types fall back to 'post'.
2052 + $expected_context = in_array($post->post_type, ['post', 'page', 'product'], true)
2053 + ? $post->post_type
2054 + : 'post';
2055 +
2056 + if ($context_type !== $expected_context) {
2057 + return new WP_Error(
2058 + 'invalid_context',
2059 + 'The context type does not match the requested content.',
2060 + ['status' => 400]
2061 + );
2062 + }
2063 +
2064 + if (!current_user_can('edit_post', $context_id)) {
2065 + return new WP_Error(
2066 + 'rest_forbidden',
2067 + 'You are not allowed to edit settings for this content.',
2068 + ['status' => 403]
2069 + );
2070 + }
2071 +
2072 + return true;
2073 + }
2074 +
2075 + /**
1755 2076 * Get arguments for validation endpoint
1756 2077 *
1757 2078 * @since 1.0.0
1758 2079 *
@@ -1951,9 +2272,27 @@
1951 2272 private function create_settings_snapshot(array $categories, string $label): string {
1952 2273 $backup_data = [];
1953 2274 foreach ($categories as $category) {
1954 2275 if (isset($this->setting_categories[$category])) {
1955 - $backup_data[$category] = $this->settings_manager->get_settings($category);
2276 + $backup_data[$category] = $this->read_category($category);
2277 + }
2278 + }
2279 +
2280 + // A snapshot that captured nothing for a category that does hold settings
2281 + // is worse than no snapshot: reset checks only that an id came back, so an
2282 + // empty one is accepted as a rollback point and the defaults go over live
2283 + // data that can no longer be recovered. That is exactly what #689 was.
2284 + //
2285 + // Ask the owning manager directly rather than trusting read_category(),
2286 + // so this stays a real check if a future edit sends a read back to the
2287 + // wrong store instead of quietly agreeing with it.
2288 + foreach ($backup_data as $category => $captured) {
2289 + if (!empty($captured) || !$this->has_seo_manager($category)) {
2290 + continue;
2291 + }
2292 +
2293 + if (!empty((array) $this->get_seo_manager($category)->get_settings('site', null))) {
2294 + return '';
1956 2295 }
1957 2296 }
1958 2297
1959 2298 $backup_metadata = [