Stop the settings sanitizer corrupting the stored client secret

CHANGELOG.md

@@ -5,6 +5,42 @@ All notable changes to Procore Connect are documented here.
 The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this
 project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [2.0.2] — 2026-08-08
+
+### Fixed
+
+- **Saved credentials could be corrupted beyond recovery.** `register_setting()`
+  attaches the settings sanitizer to `sanitize_option_{$option}`, and WordPress runs
+  that filter on *every* `update_option()` for the option — including this plugin's own
+  internal writes in `Settings::set()` and `Settings::migrate_legacy()`. The sanitizer
+  encrypted the client secret unconditionally, so each of those writes re-encrypted the
+  stored cipher text, one layer at a time. After a couple of saves the secret could no
+  longer be decrypted and the site silently lost its Procore connection, with no way to
+  recover the value short of re-entering it.
+
+  Encryption is now idempotent at that boundary: a value that is already cipher text is
+  passed through untouched. Added `Encryption::is_encrypted()` and two regression tests
+  that fail against the previous behaviour.
+
+  This was found by running the plugin in a real WordPress install with the setting
+  registered, which is the only configuration where the faulty path is reachable — the
+  earlier integration run never called `register_setting()`, so it missed it.
+
+### Added
+
+- 18 tests covering both OAuth grants and the token store: token-endpoint host, token
+  reuse, CSRF `state` rejection and replay, refresh-token rotation, the reconnect path
+  when a refresh is rejected, encryption at rest, expiry margin, environment isolation
+  and lock exclusivity.
+- `.wordpress-org/` banner and icon assets for a WordPress.org submission, excluded from
+  the release ZIP.
+
+### Removed
+
+- The `== Screenshots ==` section of `readme.txt`, which declared six files that were
+  never produced and would have rendered as six broken slots on the plugin page.
+  `.wordpress-org/README.md` records what to capture from a real Procore-connected site.
+
 ## [2.0.1] — 2026-08-08
 
 Four defects found by running the plugin inside a real WordPress install
@@ -152,6 +188,7 @@ every install was non-functional regardless of configuration.
 
 - Initial release.
 
+[2.0.2]: https://github.com/ibuilder/ProcoreWP/releases/tag/v2.0.2
 [2.0.1]: https://github.com/ibuilder/ProcoreWP/releases/tag/v2.0.1
 [2.0.0]: https://github.com/ibuilder/ProcoreWP/releases/tag/v2.0.0
 [1.0.0]: https://github.com/ibuilder/ProcoreWP/releases/tag/v1.0.0

languages/procore-connect.pot

@@ -575,7 +575,7 @@ msgstr ""
 msgid "No project was available to test against."
 msgstr ""
 
-#: src/Frontend/Shortcodes/AbstractShortcode.php:392
+#: src/Frontend/Shortcodes/AbstractShortcode.php:395
 msgid "No records were found."
 msgstr ""
 
@@ -719,7 +719,7 @@ msgid "Procore Connect"
 msgstr ""
 
 #. translators: %s: error message from the Procore API.
-#: src/Frontend/Shortcodes/AbstractShortcode.php:359
+#: src/Frontend/Shortcodes/AbstractShortcode.php:362
 msgid "Procore Connect (visible to administrators only): %s"
 msgstr ""
 
@@ -817,7 +817,7 @@ msgstr ""
 msgid "Project detail"
 msgstr ""
 
-#: src/Frontend/Shortcodes/AbstractShortcode.php:366
+#: src/Frontend/Shortcodes/AbstractShortcode.php:369
 #: src/Rest/Controller.php:209
 msgid "Project information is temporarily unavailable."
 msgstr ""

procore-connect.php

@@ -3,7 +3,7 @@
  * Plugin Name:       Procore Connect
  * Plugin URI:        https://github.com/ibuilder/ProcoreWP
  * Description:       Connect WordPress to the Procore construction management platform. Display projects, teams, drawings, RFIs and more with shortcodes, blocks and a cached REST proxy.
- * Version:           2.0.1
+ * Version:           2.0.2
  * Requires at least: 6.5
  * Requires PHP:      7.4
  * Author:            ibuilder
@@ -22,7 +22,7 @@
 
 defined( 'ABSPATH' ) || exit;
 
-const VERSION = '2.0.1';
+const VERSION = '2.0.2';
 
 define( 'PROCORE_CONNECT_VERSION', VERSION );
 define( 'PROCORE_CONNECT_FILE', __FILE__ );

readme.txt

@@ -4,7 +4,7 @@ Tags: procore, construction, project management, shortcode, api
 Requires at least: 6.5
 Tested up to: 7.0
 Requires PHP: 7.4
-Stable tag: 2.0.1
+Stable tag: 2.0.2
 License: GPLv2 or later
 License URI: https://www.gnu.org/licenses/gpl-2.0.html
 
@@ -127,6 +127,13 @@ Yes. Caching goes through the transient API, so a persistent object cache such a
 
 == Changelog ==
 
+= 2.0.2 =
+
+* **Fixed a credential-corrupting bug.** WordPress runs a registered setting's sanitize callback on every update to that option, including the plugin's own internal writes. Because the sanitizer encrypted the client secret unconditionally, each write re-encrypted the stored value until it could no longer be decrypted and the site silently lost its Procore connection. Encryption is now idempotent. If your connection stopped working after saving settings, re-enter the Client Secret once on 2.0.2 and it will stay valid.
+* Added 18 tests covering both OAuth grants, CSRF state handling and refresh-token rotation.
+* Added WordPress.org banner and icon assets.
+* Removed a Screenshots section that referenced files which did not exist.
+
 = 2.0.1 =
 
 Fixes found by running the plugin inside a real WordPress install.
@@ -167,6 +174,9 @@ A complete rewrite. See the upgrade notice below before updating.
 
 == Upgrade Notice ==
 
+= 2.0.2 =
+Important fix: repeated settings saves could re-encrypt the stored Client Secret until it became unrecoverable, silently breaking the Procore connection. Update, then re-enter your Client Secret once if the connection had stopped working.
+
 = 2.0.1 =
 Bug fixes from real-world integration testing: pagination on single-record endpoints, the REST proxy ignoring the default company, translations loading too early, and WP-CLI subcommand names. No action required.
 

src/Admin/Settings.php

@@ -221,12 +221,24 @@ public static function sanitize( $input ): array {
 
 		$clean['client_id'] = sanitize_text_field( (string) ( $input['client_id'] ?? '' ) );
 
-		// An empty secret field means "leave the stored secret alone"; the form
-		// renders a masked placeholder rather than the real value.
+		/*
+		 * An empty secret field means "leave the stored secret alone"; the form
+		 * renders a masked placeholder rather than the real value.
+		 *
+		 * A value that is already cipher text is passed through untouched.
+		 * register_setting() attaches this sanitizer to
+		 * sanitize_option_{$option}, which WordPress runs on *every*
+		 * update_option() for the option — including this plugin's own internal
+		 * writes in set() and migrate_legacy(). Encrypting unconditionally would
+		 * re-encrypt the stored value on each save, one layer at a time, until
+		 * the credential could no longer be recovered.
+		 */
 		$submitted_secret = trim( (string) ( $input['client_secret'] ?? '' ) );
 
 		if ( '' !== $submitted_secret ) {
-			$clean['client_secret'] = Encryption::encrypt( $submitted_secret );
+			$clean['client_secret'] = Encryption::is_encrypted( $submitted_secret )
+				? $submitted_secret
+				: Encryption::encrypt( $submitted_secret );
 		}
 
 		if ( ! empty( $input['clear_client_secret'] ) ) {

src/Support/Encryption.php

@@ -52,6 +52,23 @@ public static function is_strong(): bool {
 			&& in_array( self::CIPHER, (array) openssl_get_cipher_methods(), true );
 	}
 
+	/**
+	 * Whether a value has already been through `encrypt()`.
+	 *
+	 * Callers use this to stay idempotent. `register_setting()` attaches the
+	 * settings sanitizer to `sanitize_option_{$option}`, and WordPress runs
+	 * that filter on *every* `update_option()` for the option — including the
+	 * plugin's own internal writes. Encrypting unconditionally would therefore
+	 * re-encrypt the stored cipher text on each save until the credential could
+	 * no longer be recovered.
+	 *
+	 * @param string $value Candidate value.
+	 * @return bool True when the value is already cipher text.
+	 */
+	public static function is_encrypted( string $value ): bool {
+		return 0 === strpos( $value, self::PREFIX ) || 0 === strpos( $value, self::FALLBACK_PREFIX );
+	}
+
 	/**
 	 * Encrypt a value for storage.
 	 *

tests/unit/SettingsTest.php

@@ -68,6 +68,51 @@ public function test_encrypts_the_client_secret(): void {
 		$this->assertSame( 'super-secret-value', Encryption::decrypt( $clean['client_secret'] ) );
 	}
 
+	/**
+	 * Sanitizing must be idempotent, so a secret survives repeated saves.
+	 *
+	 * `register_setting()` attaches this sanitizer to `sanitize_option_{$option}`,
+	 * and WordPress runs that on every `update_option()` for the option —
+	 * including the plugin's own writes in `set()` and `migrate_legacy()`.
+	 * Encrypting unconditionally re-encrypted the stored cipher text one layer
+	 * per save until the credential was unrecoverable.
+	 *
+	 * @return void
+	 */
+	public function test_sanitize_does_not_re_encrypt_an_encrypted_secret(): void {
+		$once = Settings::sanitize( array( 'client_secret' => 'super-secret-value' ) );
+
+		$this->assertSame( 'super-secret-value', Encryption::decrypt( $once['client_secret'] ) );
+
+		// Feed the sanitizer its own output, as update_option() does.
+		$twice  = Settings::sanitize( $once );
+		$thrice = Settings::sanitize( $twice );
+
+		$this->assertSame( $once['client_secret'], $twice['client_secret'] );
+		$this->assertSame( 'super-secret-value', Encryption::decrypt( $twice['client_secret'] ) );
+		$this->assertSame( 'super-secret-value', Encryption::decrypt( $thrice['client_secret'] ) );
+	}
+
+	/**
+	 * Writing settings programmatically must not corrupt the stored secret.
+	 *
+	 * @return void
+	 */
+	public function test_set_preserves_the_secret_through_the_sanitizer(): void {
+		$stored = Settings::sanitize( array( 'client_secret' => 'keep-me' ) );
+		update_option( Settings::OPTION, $stored );
+		Settings::flush();
+
+		// Simulate WordPress re-running the sanitizer on an internal write.
+		for ( $i = 0; $i < 3; $i++ ) {
+			$round = Settings::sanitize( Settings::all() );
+			update_option( Settings::OPTION, $round );
+			Settings::flush();
+		}
+
+		$this->assertSame( 'keep-me', Settings::client_secret() );
+	}
+
 	/**
 	 * An empty secret field must leave the stored secret untouched, because the
 	 * form renders a mask rather than the real value.