Commit 40707a88734 for woocommerce
commit 40707a88734c633c4fa06d0659bbacafbcc4f3d5
Author: Jaclyn Chen <watertranquil@gmail.com>
Date: Thu Jun 11 17:47:43 2026 +0800
Log POS feed .htaccess write failures and skip unreadable files (#65621)
* Log POS feed .htaccess write failures and never overwrite unreadable files
* Add changelog entry for POS feed .htaccess hardening
* Clarify and test the missing-.htaccess creation path
* Only upgrade an existing legacy feed .htaccess, never create on missing
* Use a unique temp dir in the htaccess write-failure test
diff --git a/plugins/woocommerce/changelog/fix-pos-feed-htaccess-write-logging b/plugins/woocommerce/changelog/fix-pos-feed-htaccess-write-logging
new file mode 100644
index 00000000000..bbeb9a95d00
--- /dev/null
+++ b/plugins/woocommerce/changelog/fix-pos-feed-htaccess-write-logging
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Log when the POS product feed directory's .htaccess cannot be updated, and avoid overwriting an unreadable or custom file during the legacy upgrade.
diff --git a/plugins/woocommerce/src/Internal/ProductFeed/Storage/JsonFileFeed.php b/plugins/woocommerce/src/Internal/ProductFeed/Storage/JsonFileFeed.php
index c2b896b94a7..c5c3209affc 100644
--- a/plugins/woocommerce/src/Internal/ProductFeed/Storage/JsonFileFeed.php
+++ b/plugins/woocommerce/src/Internal/ProductFeed/Storage/JsonFileFeed.php
@@ -311,18 +311,19 @@ class JsonFileFeed implements FeedInterface {
}
/**
- * Ensures an existing feed directory allows file access while preventing directory listing.
+ * Upgrades a legacy `deny from all` .htaccess in an existing feed directory to allow file access.
*
- * Installs created before file access was enabled have a `deny from all` .htaccess in this
- * directory, which blocks feed downloads. This replaces only that known legacy directive (or
- * recreates a missing file); any other content — the already-correct directive, or custom rules
- * a site or host may have added — is left untouched.
+ * Installs created before file access was enabled have a `deny from all` .htaccess here, which
+ * blocks feed downloads. This upgrades only that known legacy directive, in place. Anything else
+ * — an already-correct directive, custom rules a site or host added, a file we cannot read, or a
+ * missing file — is left untouched. (The directory's initial .htaccess is written when the
+ * directory is first created, by `mkdir_p_not_indexable()`.)
*
* Native file functions are used here (like the feed writes elsewhere in this class) rather
* than WP_Filesystem: the directory is local, and routing through a possibly FTP/SSH-backed
* filesystem could fail to initialize and leave the old `deny from all` in place even though
- * the feed file itself was written natively. Failures are ignored so this can never interrupt
- * feed generation.
+ * the feed file itself was written natively. A failure is ignored (and logged) so it can never
+ * interrupt feed generation.
*
* @param string $directory_path The feed directory path (trailing-slashed).
* @return void
@@ -330,17 +331,33 @@ class JsonFileFeed implements FeedInterface {
private function ensure_feed_dir_file_access( string $directory_path ): void {
$htaccess_path = $directory_path . '.htaccess';
+ // Only act on an existing file. A missing .htaccess does not block downloads, so there is
+ // nothing to fix — and we should not create a file the directory did not already have.
+ if ( ! is_file( $htaccess_path ) ) {
+ return;
+ }
+
// phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged
- $current_content = is_file( $htaccess_path ) ? trim( (string) @file_get_contents( $htaccess_path ) ) : '';
+ $current_content = @file_get_contents( $htaccess_path );
- // Only upgrade the known legacy `deny from all` directive or recreate a missing file.
- // Leave anything else (already correct, or custom rules) untouched.
- if ( '' !== $current_content && FilesystemUtil::HTACCESS_DENY_ALL !== $current_content ) {
+ // Upgrade only the known legacy `deny from all` directive. Leave anything else — already
+ // correct, custom rules, or a file we cannot read — untouched, never clobbering content
+ // we did not write.
+ if ( false === $current_content || FilesystemUtil::HTACCESS_DENY_ALL !== trim( $current_content ) ) {
return;
}
- // Best effort: a failure here must never interrupt feed generation.
+ // Best effort: a failure must never interrupt feed generation, but log it — otherwise the
+ // feed would silently stay 403 behind the stale rule.
// phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged
- @file_put_contents( $htaccess_path, FilesystemUtil::HTACCESS_ALLOW_FILE_ACCESS );
+ if ( false === @file_put_contents( $htaccess_path, FilesystemUtil::HTACCESS_ALLOW_FILE_ACCESS ) ) {
+ wc_get_logger()->warning(
+ 'Could not update the product feed .htaccess to allow file access; generated feeds may remain inaccessible.',
+ array(
+ 'source' => 'product-feed',
+ 'path' => $htaccess_path,
+ )
+ );
+ }
}
}
diff --git a/plugins/woocommerce/tests/php/src/Internal/ProductFeed/Storage/JsonFileFeedTest.php b/plugins/woocommerce/tests/php/src/Internal/ProductFeed/Storage/JsonFileFeedTest.php
index 9dd0feb5aa1..aceadc708fb 100644
--- a/plugins/woocommerce/tests/php/src/Internal/ProductFeed/Storage/JsonFileFeedTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/ProductFeed/Storage/JsonFileFeedTest.php
@@ -4,6 +4,7 @@ declare( strict_types = 1 );
namespace Automattic\WooCommerce\Tests\Internal\ProductFeed\Storage;
use Automattic\WooCommerce\Internal\ProductFeed\Storage\JsonFileFeed;
+use Automattic\WooCommerce\RestApi\UnitTests\LoggerSpyTrait;
// This file works directly with local files. That's fine.
// phpcs:disable WordPress.WP.AlternativeFunctions
@@ -17,6 +18,8 @@ if ( ! function_exists( 'WP_Filesystem' ) ) {
* JsonFileFeedTest class.
*/
class JsonFileFeedTest extends \WC_Unit_Test_Case {
+ use LoggerSpyTrait;
+
/**
* Clean up test fixtures.
*/
@@ -270,6 +273,70 @@ class JsonFileFeedTest extends \WC_Unit_Test_Case {
);
}
+ /**
+ * @testdox Should leave the feed directory alone when it has no .htaccess.
+ */
+ public function test_existing_feed_dir_without_htaccess_is_left_alone(): void {
+ // Directory exists but its .htaccess is missing (e.g. it was removed) — the is_file() === false case.
+ // A missing file is not a blocked feed, so the refresh must not create one.
+ $directory = wp_upload_dir()['basedir'] . '/product-feeds';
+ wp_mkdir_p( $directory );
+ if ( file_exists( $directory . '/.htaccess' ) ) {
+ unlink( $directory . '/.htaccess' );
+ }
+
+ $feed = new JsonFileFeed( 'test-feed' );
+ $feed->start();
+ $feed->end();
+ $feed->get_file_url();
+
+ $this->assertFileDoesNotExist(
+ $directory . '/.htaccess',
+ 'A missing .htaccess must be left alone, not created by the refresh.'
+ );
+ }
+
+ /**
+ * @testdox Should log a warning when an existing legacy .htaccess cannot be overwritten.
+ */
+ public function test_logs_warning_when_htaccess_cannot_be_written(): void {
+ // Redirect uploads to a container-local path: wp-env's bind-mounted uploads ignore chmod,
+ // so the read-only legacy file must live where file permissions are actually enforced.
+ $base = get_temp_dir() . uniqid( 'wc-feed-perms-', true );
+ $feed_dir = $base . '/product-feeds';
+ $htaccess = $feed_dir . '/.htaccess';
+ mkdir( $feed_dir, 0755, true );
+ file_put_contents( $htaccess, 'deny from all' );
+ chmod( $htaccess, 0444 );
+
+ $filter = function ( $dir ) use ( $base ) {
+ $dir['basedir'] = $base;
+ $dir['baseurl'] = 'http://example.test/uploads';
+ $dir['error'] = false;
+ return $dir;
+ };
+ add_filter( 'upload_dir', $filter );
+
+ try {
+ $feed = new JsonFileFeed( 'test-feed' );
+ $feed->start();
+ $feed->end();
+ $feed->get_file_url();
+
+ $this->assertLogged(
+ 'warning',
+ 'Could not update the product feed .htaccess',
+ array( 'source' => 'product-feed' )
+ );
+ } finally {
+ remove_filter( 'upload_dir', $filter );
+ chmod( $htaccess, 0644 );
+ global $wp_filesystem;
+ WP_Filesystem();
+ $wp_filesystem->rmdir( $base, true );
+ }
+ }
+
/**
* Gets the directory for feed files, but also deletes it.
*