Derive the generated stylesheet's cache-busting version from its content

classes/helpers/FrmAppHelper.php

@@ -10,7 +10,7 @@ class FrmAppHelper {
 	 *
 	 * @var int
 	 */
-	public static $db_version = 106;
+	public static $db_version = 107;
 
 	/**
 	 * Used by the API add-on.

classes/models/FrmCreateFile.php

@@ -76,13 +76,15 @@ private function set_new_file_path( $atts ) {
 	}
 
 	/**
+	 * @since x.x Returns whether the file was written. Previously returned nothing.
+	 *
 	 * @param string $file_content
 	 *
-	 * @return void
+	 * @return bool True if the file was written to disk.
 	 */
 	public function create_file( $file_content ) {
 		if ( ! $this->has_permission ) {
-			return;
+			return false;
 		}
 
 		$dirs_exist = true;
@@ -92,11 +94,11 @@ public function create_file( $file_content ) {
 
 		// Only write the file if the folders exist.
 		if ( ! $dirs_exist ) {
-			return;
+			return false;
 		}
 
 		global $wp_filesystem;
-		$wp_filesystem->put_contents( $this->new_file_path, $file_content, $this->chmod_file );
+		return (bool) $wp_filesystem->put_contents( $this->new_file_path, $file_content, $this->chmod_file );
 	}
 
 	/**

classes/models/FrmMigrate.php

@@ -405,7 +405,7 @@ private function migrate_data( $old_db_version ) {
 			return;
 		}
 
-		$migrations = array( 16, 11, 16, 17, 23, 25, 86, 90, 97, 98, 101, 104, 105 );
+		$migrations = array( 16, 11, 16, 17, 23, 25, 86, 90, 97, 98, 101, 104, 105, 107 );
 
 		foreach ( $migrations as $migration ) {
 			if ( FrmAppHelper::$db_version < $migration || $old_db_version >= $migration ) {
@@ -484,6 +484,26 @@ public function uninstall() {
 		return true;
 	}
 
+	/**
+	 * Discard the legacy generated-stylesheet cache-busting version.
+	 *
+	 * Versions before this stored frm_last_style_update as gmdate( 'njGi' ), which could repeat
+	 * across different dates and every year, so a third-party CSS cache keyed on the enqueued
+	 * stylesheet URL could keep serving a copy generated from superseded content.
+	 *
+	 * Removing the stored value makes FrmStylesController::get_css_version() fall back to the
+	 * plugin version, which guarantees the enqueued URL changes on this upgrade even when the
+	 * post-upgrade style regeneration is skipped. The content-derived value is written by the
+	 * next FrmStyle::save_settings() call.
+	 *
+	 * @since x.x
+	 *
+	 * @return void
+	 */
+	private function migrate_to_107() {
+		delete_option( 'frm_last_style_update' );
+	}
+
 	/**
 	 * In older versions of Lite, it's possible we've saved the wrong location ID.
 	 * So force it to get valid values again.

classes/models/FrmStyle.php

@@ -407,20 +407,59 @@ public function get_color_settings() {
 	 */
 	public function save_settings() {
 		$filename = FrmAppHelper::plugin_path() . '/css/custom_theme.css.php';
-		update_option( 'frm_last_style_update', gmdate( 'njGi' ) );
 
 		if ( ! is_file( $filename ) ) {
 			return;
 		}
 
 		$this->clear_cache();
 
-		$css         = $this->get_css_content( $filename );
-		$create_file = new FrmCreateFile( self::get_create_style_file_args() );
-		$create_file->create_file( $css );
+		$css          = $this->get_css_content( $filename );
+		$create_file  = new FrmCreateFile( self::get_create_style_file_args() );
+		$file_written = $create_file->create_file( $css );
 
 		update_option( 'frmpro_css', $css, false );
 		set_transient( 'frmpro_css', $css, MONTH_IN_SECONDS );
+
+		if ( $file_written ) {
+			self::update_css_version( $css );
+		}
+	}
+
+	/**
+	 * Store the cache-busting version for the generated stylesheet.
+	 *
+	 * The value is derived from the stylesheet contents rather than the clock, so it changes if
+	 * and only if the generated bytes change. It is read back by
+	 * FrmStylesController::get_css_version() and appended to the enqueued stylesheet URL.
+	 *
+	 * This previously used gmdate( 'njGi' ), which omitted the year and concatenated unpadded
+	 * month, day and hour values. Distinct dates therefore produced identical version strings
+	 * (1 Jan 10:59, 11 Jan 00:59 and 1 Nov 00:59 all produced "111059"), the value repeated
+	 * every year, and two saves within the same minute were indistinguishable. Third-party CSS
+	 * caches keyed on the enqueued URL could keep serving a copy generated from superseded
+	 * content.
+	 *
+	 * Only called once FrmCreateFile::create_file() confirms the bytes reached disk, and only
+	 * after the frmpro_css option and transient are stored, so the version never advertises
+	 * content that is not being served. This matters because the value is content-derived and
+	 * therefore idempotent: publishing a hash for a write that silently failed would pin a
+	 * third-party cache to the superseded file permanently, since every later save of the same
+	 * content would reproduce the same hash and the same URL.
+	 *
+	 * @since x.x
+	 *
+	 * @param string $css Generated stylesheet contents.
+	 *
+	 * @return void
+	 */
+	private static function update_css_version( $css ) {
+		// skipcq: PHP-A1004 -- md5() here is a content-derived cache-busting token, not a
+		// password hash. It must be deterministic so the same stylesheet always yields the
+		// same URL; password_hash() is salted and non-deterministic and would defeat the
+		// whole mechanism. Matches the existing md5()-for-cache-key calls in FrmAddon,
+		// FrmAntiSpam, FrmFormApi and FrmStyleApi.
+		update_option( 'frm_last_style_update', substr( md5( $css ), 0, 12 ) );
 	}
 
 	/**

phpcs.xml

@@ -202,6 +202,16 @@
 	</rule>
 	<rule ref="WordPressVIPMinimum.Functions.RestrictedFunctions.file_ops_unlink">
 		<exclude-pattern>test_FrmCSVExportHelper.php</exclude-pattern>
+		<exclude-pattern>test_FrmCreateFile.php</exclude-pattern>
+		<exclude-pattern>test_FrmStyle.php</exclude-pattern>
+	</rule>
+	<rule ref="WordPressVIPMinimum.Functions.RestrictedFunctions.file_ops_file_put_contents">
+		<exclude-pattern>test_FrmCreateFile.php</exclude-pattern>
+		<exclude-pattern>test_FrmStyle.php</exclude-pattern>
+	</rule>
+	<rule ref="WordPressVIPMinimum.Functions.RestrictedFunctions.directory_rmdir">
+		<exclude-pattern>test_FrmCreateFile.php</exclude-pattern>
+		<exclude-pattern>test_FrmStyle.php</exclude-pattern>
 	</rule>
 	<rule ref="WordPressVIPMinimum.Security.ExitAfterRedirect.NoExit">
 		<exclude-pattern>FrmEntriesController.php</exclude-pattern>

tests/phpunit/database/test_FrmMigrate.php

@@ -297,6 +297,44 @@ public function test_migrate_to_97() {
 		}
 	}
 
+	/**
+	 * Make sure migrate_to_107 is actually reached by the migration dispatch (migrate_data())
+	 * when upgrading from db_version 106, not just that the method works when called directly.
+	 * A migration that is registered but never dispatched is the failure mode this guards
+	 * against.
+	 *
+	 * This deliberately calls migrate_data() directly rather than the public upgrade(), because
+	 * upgrade() unconditionally regenerates the default style at the end of every call
+	 * ( $frm_style->update( 'default' ) ), which would overwrite frm_last_style_update with a
+	 * fresh value regardless of whether migrate_to_107 actually ran, masking the result.
+	 *
+	 * @covers FrmMigrate::migrate_data
+	 * @covers FrmMigrate::migrate_to_107
+	 */
+	public function test_migrate_to_107_runs_on_dispatch_from_106() {
+		update_option( 'frm_last_style_update', '111059' );
+
+		$frmdb = new FrmMigrate();
+		$this->run_private_method( array( $frmdb, 'migrate_data' ), array( 106 ) );
+
+		$this->assertFalse( get_option( 'frm_last_style_update' ), 'migrate_to_107 should have been dispatched and deleted the legacy frm_last_style_update option.' );
+	}
+
+	/**
+	 * A site already on db_version 107 (or later) must not have migrate_to_107 run again.
+	 *
+	 * @covers FrmMigrate::migrate_data
+	 * @covers FrmMigrate::migrate_to_107
+	 */
+	public function test_migrate_to_107_does_not_run_again_once_applied() {
+		update_option( 'frm_last_style_update', 'abcdef123456' );
+
+		$frmdb = new FrmMigrate();
+		$this->run_private_method( array( $frmdb, 'migrate_data' ), array( 107 ) );
+
+		$this->assertSame( 'abcdef123456', get_option( 'frm_last_style_update' ), 'migrate_to_107 must not re-run once already at db_version 107.' );
+	}
+
 	/**
 	 * @covers FrmMigrate::collation
 	 */

tests/phpunit/misc/test_FrmCreateFile.php

@@ -0,0 +1,160 @@
+<?php
+
+/**
+ * @group misc
+ */
+class test_FrmCreateFile extends FrmUnitTest {
+
+	/**
+	 * @var array<string>
+	 */
+	private $paths_to_clean_up = array();
+
+	public function tearDown(): void {
+		foreach ( $this->paths_to_clean_up as $path ) {
+			if ( is_dir( $path ) ) {
+				@rmdir( $path ); // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged
+			} elseif ( file_exists( $path ) ) {
+				@unlink( $path ); // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged
+			}
+		}
+		$this->paths_to_clean_up = array();
+
+		parent::tearDown();
+	}
+
+	/**
+	 * @covers FrmCreateFile::create_file
+	 */
+	public function test_create_file_returns_true_on_successful_write() {
+		$uploads   = wp_upload_dir();
+		$folder    = 'frm-test-create-file-' . wp_generate_password( 8, false );
+		$file_name = 'test.css';
+
+		$create_file = new FrmCreateFile(
+			array(
+				'file_name'   => $file_name,
+				'folder_name' => $folder,
+			)
+		);
+
+		$content                   = 'body{color:#123456}';
+		$result                    = $create_file->create_file( $content );
+		$written_path              = $uploads['basedir'] . '/' . $folder . '/' . $file_name;
+		$this->paths_to_clean_up[] = $written_path;
+		$this->paths_to_clean_up[] = $uploads['basedir'] . '/' . $folder . '/index.php';
+		$this->paths_to_clean_up[] = $uploads['basedir'] . '/' . $folder;
+
+		$this->assertTrue( $result, 'create_file() should return true when the file is actually written to disk.' );
+		$this->assertSame( $content, file_get_contents( $written_path ) ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents
+	}
+
+	/**
+	 * The write-permission early return (`! $this->has_permission`) can only be reached in this
+	 * harness by forcing the private flag directly: the local/direct filesystem method always
+	 * succeeds here, and there is no practical, non-flaky way to make WP_Filesystem's credential
+	 * check fail deterministically in an automated integration run. Reflection on the private
+	 * property is used only for this one guard clause, not for the method under test itself.
+	 *
+	 * @covers FrmCreateFile::create_file
+	 */
+	public function test_create_file_returns_false_without_permission() {
+		$create_file = new FrmCreateFile(
+			array(
+				'file_name'   => 'no-permission.css',
+				'folder_name' => 'frm-test-no-permission',
+			)
+		);
+
+		$permission_property = $this->get_accessible_property( $create_file, 'has_permission' );
+		$permission_property->setValue( $create_file, false );
+
+		$result = $create_file->create_file( 'body{color:#000}' );
+
+		$this->assertFalse( $result, 'create_file() should return false when there is no filesystem permission.' );
+	}
+
+	/**
+	 * Forces the directory-creation early return (`! $dirs_exist`) with a real filesystem
+	 * collision rather than a mock: a plain file is created where FrmCreateFile needs to create a
+	 * directory of the same name, so both mkdir() and the is_dir() fallback genuinely fail.
+	 *
+	 * @covers FrmCreateFile::create_file
+	 */
+	public function test_create_file_returns_false_when_directory_cannot_be_created() {
+		$blocking     = 'frm-test-blocking-' . wp_generate_password( 8, false );
+		$blocked_path = wp_upload_dir()['basedir'] . '/' . $blocking;
+
+		$this->assertNotFalse(
+			file_put_contents( $blocked_path, 'not a directory' ), // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_read_file_put_contents
+			'Test setup: failed to create the blocking file.'
+		);
+		$this->paths_to_clean_up[] = $blocked_path;
+
+		$create_file = new FrmCreateFile(
+			array(
+				'file_name'   => 'blocked.css',
+				'folder_name' => $blocking,
+			)
+		);
+
+		$result = $create_file->create_file( 'body{color:#000}' );
+
+		$this->assertFalse( $result, 'create_file() should return false when its target directory cannot be created.' );
+	}
+
+	/**
+	 * The widened create_file() return type (void -> bool) must not affect its two internal
+	 * callers: both discard the return value and remain void themselves.
+	 *
+	 * @covers FrmCreateFile::append_file
+	 * @covers FrmCreateFile::combine_files
+	 */
+	public function test_append_file_and_combine_files_are_unaffected_by_the_widened_return_type() {
+		$uploads = wp_upload_dir();
+		$folder  = 'frm-test-callers-' . wp_generate_password( 8, false );
+
+		$append_target = new FrmCreateFile(
+			array(
+				'file_name'   => 'append.css',
+				'folder_name' => $folder,
+			)
+		);
+
+		$append_result = $append_target->append_file( 'first-part;' );
+		$this->assertNull( $append_result, 'append_file() must remain void regardless of create_file()\'s widened return type.' );
+
+		$append_result_2 = $append_target->append_file( 'second-part;' );
+		$this->assertNull( $append_result_2 );
+
+		$appended_path             = $uploads['basedir'] . '/' . $folder . '/append.css';
+		$this->paths_to_clean_up[] = $appended_path;
+		$this->paths_to_clean_up[] = $uploads['basedir'] . '/' . $folder . '/index.php';
+		$this->paths_to_clean_up[] = $uploads['basedir'] . '/' . $folder;
+
+		$this->assertSame( 'first-part;second-part;', file_get_contents( $appended_path ) ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents
+
+		$source_a = $uploads['basedir'] . '/' . $folder . '/source-a.css';
+		$source_b = $uploads['basedir'] . '/' . $folder . '/source-b.css';
+		file_put_contents( $source_a, 'a{color:#111}' ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_read_file_put_contents
+		file_put_contents( $source_b, 'b{color:#222}' ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_read_file_put_contents
+		$this->paths_to_clean_up[] = $source_a;
+		$this->paths_to_clean_up[] = $source_b;
+
+		$combine_target = new FrmCreateFile(
+			array(
+				'file_name'   => 'combined.css',
+				'folder_name' => $folder,
+			)
+		);
+
+		$combine_result = $combine_target->combine_files( array( $source_a, $source_b ) );
+		$this->assertNull( $combine_result, 'combine_files() must remain void regardless of create_file()\'s widened return type.' );
+
+		$combined_path             = $uploads['basedir'] . '/' . $folder . '/combined.css';
+		$this->paths_to_clean_up[] = $combined_path;
+
+		// phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents
+		$this->assertSame( "a{color:#111}\nb{color:#222}\n", file_get_contents( $combined_path ) );
+	}
+}