From 9a02e0ec8710e4b704bb69049a3c026030d47081 Mon Sep 17 00:00:00 2001 From: Michael Gasperini <163041857+TheStreamCode@users.noreply.github.com> Date: Wed, 21 Oct 2026 21:57:17 +0200 Subject: [PATCH 1/4] fix(db): enforce folder and file uniqueness with a dedupe migration Add UNIQUE (parent_key, name) on folders and UNIQUE (relative_path) on files through an explicit, option-guarded migration instead of dbDelta, after merging existing duplicates: the lowest folder id survives, children, files and per-folder rules are re-pointed to it, and duplicate subtrees are rewritten onto the surviving path. parent_key stores the parent id or 0 for the root because MySQL unique indexes treat NULL values as distinct. Map duplicate inserts to 409 conflicts (folder create, upload) and to skips in the reindex path instead of surfacing a database error, and replace the auto-reindex transient with an atomic add_option claim that is released when the scan fails. --- includes/class-mstv-activator.php | 478 +++++++++++++++++++++ includes/class-mstv-bootstrap.php | 1 + includes/class-mstv-reindex-lock.php | 68 +++ includes/class-mstv-repository-files.php | 14 + includes/class-mstv-repository-folders.php | 39 +- includes/class-mstv-rest-controller.php | 38 +- tests/PDMDuplicateCreationTest.php | 306 +++++++++++++ tests/PDMMigrationDedupeTest.php | 450 +++++++++++++++++++ tests/PDMReindexLockTest.php | 55 +++ tests/PDMRestControllerTest.php | 3 +- tests/bootstrap.php | 9 +- 11 files changed, 1449 insertions(+), 12 deletions(-) create mode 100644 includes/class-mstv-reindex-lock.php create mode 100644 tests/PDMDuplicateCreationTest.php create mode 100644 tests/PDMMigrationDedupeTest.php create mode 100644 tests/PDMReindexLockTest.php diff --git a/includes/class-mstv-activator.php b/includes/class-mstv-activator.php index a1e472c..aa3eab9 100644 --- a/includes/class-mstv-activator.php +++ b/includes/class-mstv-activator.php @@ -6,6 +6,8 @@ class MSTV_Activator { private const VERSION_OPTION = 'mstv_plugin_version'; private const STORAGE_MARKER_FILE = '.mstv-storage'; + private const SCHEMA_OPTION = 'mstv_schema_migrations'; + private const MAX_DEDUPE_PASSES = 10; public static function activate(bool $networkWide = false): void { @@ -189,6 +191,482 @@ private static function create_tables(): void } self::normalize_logs_table($logs_table); + + // Unique keys are added by an explicit, option-guarded migration instead of + // dbDelta: the constraint can only be created after existing duplicate rows + // have been merged, and dbDelta would attempt the ALTER before that cleanup. + self::ensure_unique_indexes($folders_table, $files_table, $folder_permissions_table); + } + + /** + * Whether a one-time schema migration has been applied. + * + * Runtime code (folder inserts, uploads) must not reference the parent_key + * column before the migration that creates it has run. + */ + public static function schema_migration_applied(string $key): bool + { + $applied = get_option(self::SCHEMA_OPTION, []); + + return is_array($applied) && !empty($applied[$key]); + } + + private static function mark_schema_migration(string $key): void + { + $applied = get_option(self::SCHEMA_OPTION, []); + + if (!is_array($applied)) { + $applied = []; + } + + $applied[$key] = 1; + update_option(self::SCHEMA_OPTION, $applied); + } + + /** + * Add the uniqueness constraints that keep one row per folder name and per + * stored file path. + * + * The folders table gets a NULL-safe sibling key: MySQL treats NULL values as + * distinct in a unique index, so UNIQUE (parent_id, name) would not protect + * root folders. parent_key stores the parent id or 0 for the root, and + * UNIQUE (parent_key, name) covers both cases. + * + * Every step is recorded in the mstv_schema_migrations option so a retried + * activation skips work that already succeeded, and a failed step is retried + * on the next activation/upgrade instead of leaving the schema half-applied. + */ + private static function ensure_unique_indexes(string $foldersTable, string $filesTable, string $permissionsTable = ''): void + { + global $wpdb; + + if (!is_object($wpdb) || !method_exists($wpdb, 'query')) { + return; + } + + if (!self::is_safe_table_name($foldersTable) || !self::is_safe_table_name($filesTable)) { + self::log_schema_error('unexpected table name'); + return; + } + + if ($permissionsTable !== '' && !self::is_safe_table_name($permissionsTable)) { + $permissionsTable = ''; + } + + $parentKeyReady = self::schema_migration_applied('folder_parent_key'); + $folderUniqueReady = self::schema_migration_applied('folder_unique'); + $fileUniqueReady = self::schema_migration_applied('file_unique'); + + if ($parentKeyReady && $folderUniqueReady && $fileUniqueReady) { + return; + } + + if (!$parentKeyReady) { + $parentKeyReady = self::add_folder_parent_key_column($wpdb, $foldersTable); + + if ($parentKeyReady) { + self::mark_schema_migration('folder_parent_key'); + } else { + self::log_schema_error('folder parent_key column'); + } + } + + if ($parentKeyReady && !$folderUniqueReady) { + if (self::deduplicate_folders($wpdb, $foldersTable, $filesTable, $permissionsTable) + && self::add_unique_index($wpdb, $foldersTable, 'uq_mstv_folder_parent_name', 'parent_key, name(150)') + ) { + self::mark_schema_migration('folder_unique'); + } else { + self::log_schema_error('folder unique index'); + } + } + + if (!$fileUniqueReady) { + if (self::deduplicate_files($wpdb, $filesTable) + && !self::file_paths_share_index_prefix($wpdb, $filesTable) + && self::add_unique_index($wpdb, $filesTable, 'uq_mstv_file_relative_path', 'relative_path(191)') + ) { + self::mark_schema_migration('file_unique'); + } else { + self::log_schema_error('file unique index'); + } + } + } + + private static function add_folder_parent_key_column($wpdb, string $foldersTable): bool + { + if (!method_exists($wpdb, 'get_results') || !method_exists($wpdb, 'query')) { + return false; + } + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.DirectDatabaseQuery.SchemaChange,PluginCheck.Security.DirectDB.UnescapedDBParameter,WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Table name is validated against the blog-specific plugin table before interpolation. + $columns = $wpdb->get_results('DESCRIBE `' . $foldersTable . '`'); + + if (!is_array($columns)) { + return false; + } + + foreach ($columns as $column) { + if (isset($column->Field) && $column->Field === 'parent_key') { + return true; + } + } + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.DirectDatabaseQuery.SchemaChange,PluginCheck.Security.DirectDB.UnescapedDBParameter,WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Validated plugin table; explicit ALTER is required for the unique key. + $altered = $wpdb->query( + 'ALTER TABLE `' . $foldersTable . '` ADD COLUMN parent_key BIGINT(20) UNSIGNED NOT NULL DEFAULT 0' + ); + + if ($altered === false) { + return false; + } + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,PluginCheck.Security.DirectDB.UnescapedDBParameter,WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Validated plugin table; backfills the new column from the existing parent_id values. + $wpdb->query('UPDATE `' . $foldersTable . '` SET parent_key = COALESCE(parent_id, 0)'); + + return true; + } + + /** + * Merge duplicate sibling folders before the unique key is created. + * + * The lowest id survives; children and files of the duplicate are re-pointed + * to it and the stored paths of the duplicate subtree are rewritten onto the + * surviving path. Duplicates introduced by re-pointing are handled by the + * next pass, so nested duplicate trees converge. + */ + public static function deduplicate_folders($wpdb, string $foldersTable, string $filesTable, string $permissionsTable = ''): bool + { + if (!is_object($wpdb) || !method_exists($wpdb, 'get_results')) { + return false; + } + + for ($pass = 0; $pass < self::MAX_DEDUPE_PASSES; $pass++) { + $groups = self::find_duplicate_folder_groups($wpdb, $foldersTable); + + if ($groups === null) { + return false; + } + + if ($groups === []) { + return true; + } + + foreach ($groups as $group) { + $keepId = (int) $group['keep_id']; + $keepPath = (string) $group['keep_path']; + + foreach ($group['duplicate_ids'] as $duplicateId) { + $duplicateId = (int) $duplicateId; + + $wpdb->update( + $foldersTable, + ['parent_id' => $keepId, 'parent_key' => $keepId], + ['parent_id' => $duplicateId], + ['%d', '%d'], + ['%d'] + ); + $wpdb->update($filesTable, ['folder_id' => $keepId], ['folder_id' => $duplicateId], ['%d'], ['%d']); + + if ($permissionsTable !== '') { + // Per-folder rules of the duplicate belong to the surviving + // folder. UPDATE IGNORE skips rules the survivor already has; + // the following DELETE removes exactly those duplicates so no + // orphan rule survives the merge. + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,PluginCheck.Security.DirectDB.UnescapedDBParameter,WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Validated plugin table; values are prepared. + $wpdb->query( + $wpdb->prepare( + 'UPDATE IGNORE `' . $permissionsTable . '` SET folder_id = %d WHERE folder_id = %d', + $keepId, + $duplicateId + ) + ); + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,PluginCheck.Security.DirectDB.UnescapedDBParameter,WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Validated plugin table; value is prepared. + $wpdb->query( + $wpdb->prepare( + 'DELETE FROM `' . $permissionsTable . '` WHERE folder_id = %d', + $duplicateId + ) + ); + } + + $duplicatePath = (string) ($group['duplicate_paths'][$duplicateId] ?? ''); + + if ($duplicatePath !== $keepPath) { + self::rewrite_subtree_paths($wpdb, $foldersTable, $filesTable, $duplicatePath, $keepPath); + } + + $wpdb->delete($foldersTable, ['id' => $duplicateId], ['%d']); + } + } + } + + return false; + } + + /** + * Delete duplicate file rows that point at the same stored path, keeping the + * lowest id. Files are not re-pointed: the surviving row already owns the + * path, and both rows describe the same bytes on disk. + */ + public static function deduplicate_files($wpdb, string $filesTable): bool + { + if (!is_object($wpdb) || !method_exists($wpdb, 'get_results')) { + return false; + } + + $rows = self::fetch_file_rows($wpdb, $filesTable); + + if ($rows === null) { + return false; + } + + foreach (self::duplicate_file_groups($rows) as $group) { + foreach ($group['duplicate_ids'] as $duplicateId) { + $wpdb->delete($filesTable, ['id' => (int) $duplicateId], ['%d']); + } + } + + return true; + } + + /** + * @param object[] $rows Rows with id and relative_path. + * @return array + */ + public static function duplicate_file_groups(array $rows): array + { + $groups = []; + + foreach ($rows as $row) { + $groups[(string) $row->relative_path][] = (int) $row->id; + } + + $duplicates = []; + + foreach ($groups as $ids) { + if (count($ids) < 2) { + continue; + } + + sort($ids); + $keepId = array_shift($ids); + + $duplicates[] = [ + 'keep_id' => $keepId, + 'duplicate_ids' => $ids, + ]; + } + + return $duplicates; + } + + /** + * Rewrite a stored path when its folder prefix moved to the surviving path. + */ + public static function replace_path_prefix(string $path, string $oldPrefix, string $newPrefix): string + { + $oldPrefix = trim($oldPrefix, '/'); + $newPrefix = trim($newPrefix, '/'); + + if ($oldPrefix === '' || $oldPrefix === $newPrefix) { + return $path; + } + + if ($path === $oldPrefix) { + return $newPrefix; + } + + if (str_starts_with($path, $oldPrefix . '/')) { + return $newPrefix . substr($path, strlen($oldPrefix)); + } + + return $path; + } + + /** + * Find sibling folder duplicates with the same semantics as the unique key. + * + * The GROUP BY runs under the column collation, so case-insensitive and + * accent-insensitive duplicates are found exactly as the index would treat + * them; grouping in PHP would risk merging rows the database considers + * distinct. + * + * @return array}>|null + */ + private static function find_duplicate_folder_groups($wpdb, string $foldersTable): ?array + { + if (!method_exists($wpdb, 'get_results') || !method_exists($wpdb, 'get_row')) { + return null; + } + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,PluginCheck.Security.DirectDB.UnescapedDBParameter,WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Validated plugin table; one-time duplicate detection before the unique key is created. + $groups = $wpdb->get_results( + 'SELECT parent_key, name, MIN(id) AS keep_id, COUNT(*) AS total FROM `' . $foldersTable . '`' + . ' GROUP BY parent_key, name HAVING total > 1' + ); + + if (!is_array($groups)) { + return null; + } + + $duplicates = []; + + foreach ($groups as $group) { + $keepId = (int) $group->keep_id; + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,PluginCheck.Security.DirectDB.UnescapedDBParameter,WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Validated plugin table; reads the surviving row of a duplicate group. + $keep = $wpdb->get_row( + $wpdb->prepare('SELECT id, relative_path FROM `' . $foldersTable . '` WHERE id = %d', $keepId) + ); + + if (!is_object($keep)) { + return null; + } + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,PluginCheck.Security.DirectDB.UnescapedDBParameter,WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Validated plugin table; lists the duplicate rows of one group. + $rows = $wpdb->get_results( + $wpdb->prepare( + 'SELECT id, relative_path FROM `' . $foldersTable . '`' + . ' WHERE parent_key = %d AND name = %s AND id <> %d ORDER BY id ASC', + (int) $group->parent_key, + (string) $group->name, + $keepId + ) + ); + + if (!is_array($rows) || $rows === []) { + continue; + } + + $duplicateIds = []; + $duplicatePaths = []; + + foreach ($rows as $row) { + $duplicateIds[] = (int) $row->id; + $duplicatePaths[(int) $row->id] = (string) $row->relative_path; + } + + $duplicates[] = [ + 'keep_id' => $keepId, + 'keep_path' => (string) $keep->relative_path, + 'duplicate_ids' => $duplicateIds, + 'duplicate_paths' => $duplicatePaths, + ]; + } + + return $duplicates; + } + + /** + * @return object[]|null + */ + private static function fetch_file_rows($wpdb, string $table): ?array + { + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,PluginCheck.Security.DirectDB.UnescapedDBParameter,WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Validated plugin table; the migration reads every row once to merge duplicates. + $rows = $wpdb->get_results('SELECT id, relative_path FROM `' . $table . '` ORDER BY id ASC'); + + return is_array($rows) ? $rows : null; + } + + private static function rewrite_subtree_paths($wpdb, string $foldersTable, string $filesTable, string $oldPrefix, string $newPrefix): void + { + $oldPrefix = trim($oldPrefix, '/'); + $newPrefix = trim($newPrefix, '/'); + + if ($oldPrefix === '' || $oldPrefix === $newPrefix) { + return; + } + + foreach ([$foldersTable, $filesTable] as $table) { + $like = (method_exists($wpdb, 'esc_like') ? $wpdb->esc_like($oldPrefix) : $oldPrefix) . '/%'; + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,PluginCheck.Security.DirectDB.UnescapedDBParameter,WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Validated plugin table; prefix match limits the rewrite to the duplicate subtree. + $rows = $wpdb->get_results( + $wpdb->prepare( + 'SELECT id, relative_path FROM `' . $table . '` WHERE relative_path = %s OR relative_path LIKE %s', + $oldPrefix, + $like + ) + ); + + if (!is_array($rows)) { + continue; + } + + foreach ($rows as $row) { + $newPath = self::replace_path_prefix((string) $row->relative_path, $oldPrefix, $newPrefix); + + if ($newPath === (string) $row->relative_path) { + continue; + } + + $wpdb->update($table, ['relative_path' => $newPath], ['id' => (int) $row->id], ['%s'], ['%d']); + } + } + } + + /** + * A unique index on relative_path uses a 191-character prefix for MySQL + * compatibility. Distinct paths sharing that prefix cannot coexist under the + * index, so the index is skipped (and retried on the next upgrade) rather + * than deleting valid file rows. + */ + private static function file_paths_share_index_prefix($wpdb, string $filesTable): bool + { + if (!method_exists($wpdb, 'get_var')) { + return true; + } + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,PluginCheck.Security.DirectDB.UnescapedDBParameter,WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Validated plugin table; detects prefix collisions before creating the unique index. + $collisions = $wpdb->get_var( + 'SELECT COUNT(*) FROM (SELECT LEFT(relative_path, 191) AS path_prefix, COUNT(DISTINCT relative_path) AS variants' + . ' FROM `' . $filesTable . '` GROUP BY path_prefix HAVING variants > 1) AS collisions' + ); + + return (int) $collisions > 0; + } + + private static function add_unique_index($wpdb, string $table, string $indexName, string $columns): bool + { + if (!method_exists($wpdb, 'get_var') || !method_exists($wpdb, 'query')) { + return false; + } + + $indexName = preg_replace('/[^A-Za-z0-9_]/', '', $indexName); + + if ($indexName === '') { + return false; + } + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,PluginCheck.Security.DirectDB.UnescapedDBParameter,WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Validated plugin table; index presence check. + $existing = $wpdb->get_var( + $wpdb->prepare( + 'SELECT COUNT(*) FROM information_schema.statistics WHERE table_schema = DATABASE() AND table_name = %s AND index_name = %s', + $table, + $indexName + ) + ); + + if ((int) $existing > 0) { + return true; + } + + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.DirectDatabaseQuery.SchemaChange,PluginCheck.Security.DirectDB.UnescapedDBParameter,WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- Explicit ALTER is required for the unique key; dbDelta cannot add it after the dedupe. + $altered = $wpdb->query('ALTER TABLE `' . $table . '` ADD UNIQUE KEY `' . $indexName . '` (' . $columns . ')'); + + return $altered !== false; + } + + private static function is_safe_table_name(string $table): bool + { + return (bool) preg_match('/^[A-Za-z0-9_]+$/', $table); + } + + private static function log_schema_error(string $context): void + { + // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log -- A failed one-time schema migration needs operator attention; no user data is logged. + error_log('TeamVault: unable to apply the uniqueness migration (' . $context . ').'); } private static function normalize_logs_table(string $logsTable): void diff --git a/includes/class-mstv-bootstrap.php b/includes/class-mstv-bootstrap.php index 7358f4f..42b8056 100644 --- a/includes/class-mstv-bootstrap.php +++ b/includes/class-mstv-bootstrap.php @@ -50,6 +50,7 @@ private function load_dependencies(): void 'class-mstv-repository-permissions', 'class-mstv-permissions', 'class-mstv-quota', + 'class-mstv-reindex-lock', 'class-mstv-notifications', 'trait-mstv-binary-stream', 'class-mstv-download', diff --git a/includes/class-mstv-reindex-lock.php b/includes/class-mstv-reindex-lock.php new file mode 100644 index 0000000..e18c73f --- /dev/null +++ b/includes/class-mstv-reindex-lock.php @@ -0,0 +1,68 @@ +blogId = $blogId !== null && $blogId > 0 ? $blogId : (int) get_current_blog_id(); + } + + public function option_name(): string + { + return self::OPTION_PREFIX . $this->blogId; + } + + /** + * Try to become the reindex owner. + * + * Returns false while another request holds a fresh claim. A stale claim + * (older than TTL) is taken over so a crashed request cannot block the + * self-healing path forever; add_option() still admits exactly one owner. + */ + public function acquire(): bool + { + $now = time(); + + if (add_option($this->option_name(), (string) $now, '', false)) { + return true; + } + + $claimedAt = (int) get_option($this->option_name(), 0); + + if ($claimedAt > 0 && ($now - $claimedAt) < self::TTL) { + return false; + } + + delete_option($this->option_name()); + + return (bool) add_option($this->option_name(), (string) $now, '', false); + } + + /** + * Release the claim so the next request can retry immediately. A successful + * scan keeps the claim until it goes stale, which throttles repeated scans + * of a storage directory whose files are all rejected by the upload policy. + */ + public function release(): void + { + delete_option($this->option_name()); + } +} diff --git a/includes/class-mstv-repository-files.php b/includes/class-mstv-repository-files.php index 6f53775..b4f5d13 100644 --- a/includes/class-mstv-repository-files.php +++ b/includes/class-mstv-repository-files.php @@ -222,6 +222,20 @@ public function create(array $data): int return $inserted === false ? 0 : (int) $wpdb->insert_id; } + public function find_by_relative_path(string $relativePath): ?object + { + global $wpdb; + + $result = $wpdb->get_row( + $wpdb->prepare( + "SELECT * FROM {$this->table} WHERE relative_path = %s ORDER BY id ASC LIMIT 1", + $relativePath + ) + ); + + return is_object($result) ? $result : null; + } + public function update(int $id, array $data): bool { global $wpdb; diff --git a/includes/class-mstv-repository-folders.php b/includes/class-mstv-repository-folders.php index b8edc5f..d15fe71 100644 --- a/includes/class-mstv-repository-folders.php +++ b/includes/class-mstv-repository-folders.php @@ -96,13 +96,21 @@ public function create(array $data): int { global $wpdb; - $inserted = $wpdb->insert($this->table, [ + $record = [ 'parent_id' => $data['parent_id'] ?? null, 'name' => $data['name'], 'slug' => $data['slug'], 'relative_path' => $data['relative_path'], 'created_by' => $data['created_by'], - ], ['%d', '%s', '%s', '%s', '%d']); + ]; + $formats = ['%d', '%s', '%s', '%s', '%d']; + + if (self::parent_key_enabled()) { + $record['parent_key'] = (int) ($data['parent_id'] ?? 0); + $formats[] = '%d'; + } + + $inserted = $wpdb->insert($this->table, $record, $formats); $this->flush_cache(); @@ -113,6 +121,10 @@ public function update(int $id, array $data): bool { global $wpdb; + if (self::parent_key_enabled() && array_key_exists('parent_id', $data)) { + $data['parent_key'] = (int) ($data['parent_id'] ?? 0); + } + $result = $wpdb->update( $this->table, $data, @@ -139,6 +151,20 @@ public function delete(int $id): bool return $result !== false; } + public function find_by_relative_path(string $relativePath): ?object + { + global $wpdb; + + $result = $wpdb->get_row( + $wpdb->prepare( + "SELECT * FROM {$this->table} WHERE relative_path = %s ORDER BY id ASC LIMIT 1", + $relativePath + ) + ); + + return is_object($result) ? $result : null; + } + public function exists_by_name_and_parent(string $name, ?int $parentId): bool { global $wpdb; @@ -228,6 +254,15 @@ private function flush_cache(): void $this->allFoldersCache = null; $this->hierarchyCache = null; } + + /** + * Whether the parent_key column exists. The uniqueness migration adds it, and + * inserts must not reference it before that migration has run. + */ + private static function parent_key_enabled(): bool + { + return class_exists('MSTV_Activator') && MSTV_Activator::schema_migration_applied('folder_parent_key'); + } } // phpcs:enable WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.InterpolatedNotPrepared diff --git a/includes/class-mstv-rest-controller.php b/includes/class-mstv-rest-controller.php index 0c2c8c4..20e5c8c 100644 --- a/includes/class-mstv-rest-controller.php +++ b/includes/class-mstv-rest-controller.php @@ -7,7 +7,6 @@ class MSTV_REST_Controller private const NAMESPACE = 'mstv/v1'; private const DEFAULT_PER_PAGE = 50; private const MAX_PER_PAGE = 200; - private const AUTO_REINDEX_TTL = 300; private MSTV_Settings $settings; private MSTV_Auth $auth; @@ -630,7 +629,7 @@ public function create_folder(\WP_REST_Request $request): \WP_REST_Response|\WP_ } if ($this->folderRepo->exists_by_name_and_parent($name, $parentId)) { - return new \WP_Error('duplicate_error', __('Folder already exists.', 'mikesoft-teamvault'), ['status' => 400]); + return new \WP_Error('duplicate_error', __('Folder already exists.', 'mikesoft-teamvault'), ['status' => 409]); } $result = $this->storage->create_folder($name, $parentId, $this->folderRepo); @@ -647,6 +646,13 @@ public function create_folder(\WP_REST_Request $request): \WP_REST_Response|\WP_ ]); if ($folderId <= 0) { + // A concurrent request created the same sibling first and the unique key + // rejected this row. Keep the directory, which now belongs to the + // surviving folder, and report a conflict instead of a database error. + if ($this->folderRepo->find_by_relative_path((string) $result['relative_path']) !== null) { + return new \WP_Error('duplicate_error', __('Folder already exists.', 'mikesoft-teamvault'), ['status' => 409]); + } + $this->storage->get_filesystem()->delete_directory((string) $result['relative_path']); return $this->database_write_error(); } @@ -994,8 +1000,11 @@ public function upload_file(\WP_REST_Request $request): \WP_REST_Response|\WP_Er $result = $this->storage->store_uploaded_file($files, $folderId, $this->folderRepo); if (!$result['success']) { + $errorCode = isset($result['error_code']) && is_string($result['error_code']) + ? $result['error_code'] + : 'storage_error'; ob_end_clean(); - return new \WP_Error('storage_error', $result['error'], ['status' => 500]); + return new \WP_Error($errorCode, $result['error'], ['status' => 500]); } $storedFileSize = !empty($result['file_size']) ? (int) $result['file_size'] : (int) $validation['size']; @@ -1014,6 +1023,18 @@ public function upload_file(\WP_REST_Request $request): \WP_REST_Response|\WP_Er ]); if ($fileId <= 0) { + // A concurrent request already owns this exact storage path: keep the + // file because the surviving row references it, and report a conflict + // instead of a database error. + if ($this->filesRepo->find_by_relative_path((string) $result['relative_path']) !== null) { + ob_end_clean(); + return new \WP_Error( + 'duplicate_file', + __('A file with this storage path already exists.', 'mikesoft-teamvault'), + ['status' => 409] + ); + } + $this->storage->get_filesystem()->delete_file((string) $result['relative_path']); ob_end_clean(); return $this->database_write_error(); @@ -1286,18 +1307,18 @@ private function with_no_store(\WP_REST_Response $response): \WP_REST_Response private function maybe_auto_restore_storage_index(): void { - $transientKey = 'mstv_auto_reindex_' . get_current_blog_id(); + $lock = new MSTV_Reindex_Lock(); - if (get_transient($transientKey)) { + if (!$lock->acquire()) { return; } - set_transient($transientKey, 1, self::AUTO_REINDEX_TTL); - // Normal create/upload/move/delete operations keep the database in sync, so a full // recursive storage scan is only needed to self-heal an empty index (for example after // the database was reset or migrated without its records). Once the index is populated // we skip the scan, keeping it off the request path during normal use on large vaults. + // The claim stays until it goes stale, which throttles repeated scans when every stored + // file is rejected by the upload policy. if ($this->filesRepo->get_count() > 0 || $this->folderRepo->find_all() !== []) { return; } @@ -1313,7 +1334,8 @@ private function maybe_auto_restore_storage_index(): void ); if (empty($result['success'])) { - delete_transient($transientKey); + // Release the claim so the next request can retry the self-healing scan. + $lock->release(); } } diff --git a/tests/PDMDuplicateCreationTest.php b/tests/PDMDuplicateCreationTest.php new file mode 100644 index 0000000..ed70f43 --- /dev/null +++ b/tests/PDMDuplicateCreationTest.php @@ -0,0 +1,306 @@ +storagePath = sys_get_temp_dir() . '/mstv-duplicate-test-' . bin2hex(random_bytes(6)); + } + + protected function tearDown(): void + { + unset($GLOBALS['wp_filesystem'], $GLOBALS['wpdb']); + $_FILES = []; + $GLOBALS['pdm_test_options'] = []; + + $this->deleteDirectory($this->storagePath); + } + + public function test_create_folder_reports_conflict_when_the_sibling_exists(): void + { + $folderRepo = $this->getMockBuilder(MSTV_Repository_Folders::class)->disableOriginalConstructor()->getMock(); + $folderRepo->method('exists_by_name_and_parent')->willReturn(true); + + $controller = $this->buildController( + $folderRepo, + $this->getMockBuilder(MSTV_Storage::class)->disableOriginalConstructor()->getMock(), + $this->getMockBuilder(MSTV_Repository_Files::class)->disableOriginalConstructor()->getMock() + ); + + $result = $controller->create_folder(new WP_REST_Request(['name' => 'Docs', 'parent_id' => null])); + + self::assertInstanceOf(WP_Error::class, $result); + self::assertSame('duplicate_error', $result->get_error_code()); + self::assertSame(409, $result->data['status']); + } + + public function test_create_folder_race_maps_the_unique_violation_to_conflict(): void + { + $filesystem = $this->createMock(MSTV_Filesystem::class); + $filesystem->expects(self::never())->method('delete_directory'); + + $storage = $this->getMockBuilder(MSTV_Storage::class)->disableOriginalConstructor()->getMock(); + $storage->method('create_folder')->willReturn([ + 'success' => true, + 'slug' => 'docs', + 'relative_path' => 'docs', + ]); + $storage->method('get_filesystem')->willReturn($filesystem); + $storage->method('has_reindexable_content')->willReturn(false); + + $folderRepo = $this->getMockBuilder(MSTV_Repository_Folders::class)->disableOriginalConstructor()->getMock(); + $folderRepo->method('exists_by_name_and_parent')->willReturn(false); + $folderRepo->method('create')->willReturn(0); + $folderRepo->method('find_by_relative_path')->with('docs')->willReturn( + (object) ['id' => 7, 'relative_path' => 'docs'] + ); + + $controller = $this->buildController( + $folderRepo, + $storage, + $this->getMockBuilder(MSTV_Repository_Files::class)->disableOriginalConstructor()->getMock(), + true + ); + + $result = $controller->create_folder(new WP_REST_Request(['name' => 'Docs', 'parent_id' => null])); + + self::assertInstanceOf(WP_Error::class, $result); + self::assertSame('duplicate_error', $result->get_error_code()); + self::assertSame(409, $result->data['status']); + } + + public function test_upload_maps_a_duplicate_storage_path_to_conflict(): void + { + $_FILES['file'] = [ + 'name' => 'contract.pdf', + 'type' => 'application/pdf', + 'tmp_name' => '/tmp/contract.pdf', + 'error' => UPLOAD_ERR_OK, + 'size' => 123, + ]; + + $storage = $this->getMockBuilder(MSTV_Storage::class)->disableOriginalConstructor()->getMock(); + $storage->method('ensure_storage_directory')->willReturn(true); + $storage->method('store_uploaded_file')->willReturn([ + 'success' => true, + 'stored_name' => 'abc.pdf', + 'relative_path' => 'abc.pdf', + 'extension' => 'pdf', + 'file_size' => 123, + 'checksum' => 'checksum', + ]); + + $filesRepo = $this->getMockBuilder(MSTV_Repository_Files::class)->disableOriginalConstructor()->getMock(); + $filesRepo->method('create')->willReturn(0); + $filesRepo->method('find_by_relative_path')->with('abc.pdf')->willReturn( + (object) ['id' => 3, 'relative_path' => 'abc.pdf'] + ); + + $controller = $this->buildController( + $this->getMockBuilder(MSTV_Repository_Folders::class)->disableOriginalConstructor()->getMock(), + $storage, + $filesRepo + ); + + $result = $controller->upload_file(new WP_REST_Request()); + + self::assertInstanceOf(WP_Error::class, $result); + self::assertSame('duplicate_file', $result->get_error_code()); + self::assertSame(409, $result->data['status']); + } + + public function test_upload_reports_the_unprotected_storage_error_code(): void + { + $_FILES['file'] = [ + 'name' => 'contract.pdf', + 'type' => 'application/pdf', + 'tmp_name' => '/tmp/contract.pdf', + 'error' => UPLOAD_ERR_OK, + 'size' => 123, + ]; + + $storage = $this->getMockBuilder(MSTV_Storage::class)->disableOriginalConstructor()->getMock(); + $storage->method('ensure_storage_directory')->willReturn(true); + $storage->method('store_uploaded_file')->willReturn([ + 'success' => false, + 'error_code' => 'storage_unprotected', + 'error' => 'Upload refused: TeamVault could not create the protection files (.htaccess) in the private storage directory.', + ]); + + $controller = $this->buildController( + $this->getMockBuilder(MSTV_Repository_Folders::class)->disableOriginalConstructor()->getMock(), + $storage, + $this->getMockBuilder(MSTV_Repository_Files::class)->disableOriginalConstructor()->getMock() + ); + + $result = $controller->upload_file(new WP_REST_Request()); + + self::assertInstanceOf(WP_Error::class, $result); + self::assertSame('storage_unprotected', $result->get_error_code()); + self::assertSame(500, $result->data['status']); + } + + public function test_reindex_reuses_a_folder_row_rejected_by_the_unique_key(): void + { + $this->prepareStorageDirectory(['docs' => ['brief.txt' => 'hello']]); + + $folderRepo = $this->getMockBuilder(MSTV_Repository_Folders::class)->disableOriginalConstructor()->getMock(); + $folderRepo->method('find_all')->willReturn([]); + $folderRepo->method('create')->willReturn(0); + $folderRepo->method('find_by_relative_path')->with('docs')->willReturn( + (object) ['id' => 42, 'relative_path' => 'docs'] + ); + + $createdFiles = []; + $filesRepo = $this->getMockBuilder(MSTV_Repository_Files::class)->disableOriginalConstructor()->getMock(); + $filesRepo->method('find_all')->willReturn([]); + $filesRepo->method('find_by_relative_path')->willReturn(null); + $filesRepo->method('create')->willReturnCallback(static function (array $data) use (&$createdFiles): int { + $createdFiles[] = $data; + + return count($createdFiles); + }); + + $result = (new MSTV_Storage(new MSTV_Settings()))->reindex_storage_records($folderRepo, $filesRepo, 7); + + self::assertTrue($result['success']); + self::assertSame(0, $result['folders_created']); + self::assertSame(1, $result['files_created']); + self::assertSame(42, $createdFiles[0]['folder_id']); + } + + public function test_reindex_skips_a_file_row_rejected_by_the_unique_key(): void + { + $this->prepareStorageDirectory(['brief.txt' => 'hello']); + + $folderRepo = $this->getMockBuilder(MSTV_Repository_Folders::class)->disableOriginalConstructor()->getMock(); + $folderRepo->method('find_all')->willReturn([]); + + $filesRepo = $this->getMockBuilder(MSTV_Repository_Files::class)->disableOriginalConstructor()->getMock(); + $filesRepo->method('find_all')->willReturn([]); + $filesRepo->method('create')->willReturn(0); + $filesRepo->method('find_by_relative_path')->with('brief.txt')->willReturn( + (object) ['id' => 5, 'relative_path' => 'brief.txt'] + ); + + $result = (new MSTV_Storage(new MSTV_Settings()))->reindex_storage_records($folderRepo, $filesRepo, 7); + + self::assertTrue($result['success']); + self::assertSame(0, $result['files_created']); + self::assertSame(1, $result['files_skipped']); + } + + private function buildController( + MSTV_Repository_Folders $folderRepo, + MSTV_Storage $storage, + MSTV_Repository_Files $filesRepo, + bool $withFolderValidator = false + ): MSTV_REST_Controller { + $validator = $this->createMock(MSTV_Validator::class); + + if ($withFolderValidator) { + $validator->method('validate_folder_name')->willReturn(['valid' => true, 'errors' => []]); + } + + $validator->method('validate_upload_full')->willReturn([ + 'valid' => true, + 'errors' => [], + 'extension' => 'pdf', + 'mime_type' => 'application/pdf', + 'size' => 123, + ]); + $validator->method('validate_file_name')->willReturn(['valid' => true, 'errors' => []]); + + $auth = $this->createMock(MSTV_Auth::class); + $auth->method('get_current_user_id')->willReturn(5); + + return new MSTV_REST_Controller( + new MSTV_Settings(), + $auth, + $storage, + $validator, + $folderRepo, + $filesRepo, + $this->getMockBuilder(MSTV_Download::class)->disableOriginalConstructor()->getMock(), + $this->getMockBuilder(MSTV_Preview::class)->disableOriginalConstructor()->getMock(), + $this->createMock(MSTV_Logger::class) + ); + } + + /** + * @param array $structure Files and folders to create inside the storage root. + */ + private function prepareStorageDirectory(array $structure): void + { + if (!is_dir($this->storagePath)) { + mkdir($this->storagePath, 0777, true); + } + + file_put_contents($this->storagePath . '/.mstv-storage', "marker\n"); + file_put_contents($this->storagePath . '/.htaccess', str_repeat('x', 10)); + file_put_contents($this->storagePath . '/web.config', str_repeat('y', 10)); + file_put_contents($this->storagePath . '/index.php', ' $value) { + if (is_array($value)) { + mkdir($this->storagePath . '/' . $name, 0777, true); + + foreach ($value as $childName => $childValue) { + file_put_contents($this->storagePath . '/' . $name . '/' . $childName, (string) $childValue); + } + + continue; + } + + file_put_contents($this->storagePath . '/' . $name, (string) $value); + } + + $GLOBALS['pdm_test_options'] = ['mstv_storage_path' => $this->storagePath]; + } + + private function deleteDirectory(string $path): void + { + if (!is_dir($path)) { + return; + } + + $items = scandir($path); + + if ($items === false) { + return; + } + + foreach ($items as $item) { + if (in_array($item, ['.', '..'], true)) { + continue; + } + + $itemPath = $path . DIRECTORY_SEPARATOR . $item; + + if (is_dir($itemPath)) { + $this->deleteDirectory($itemPath); + continue; + } + + unlink($itemPath); + } + + rmdir($path); + } +} diff --git a/tests/PDMMigrationDedupeTest.php b/tests/PDMMigrationDedupeTest.php new file mode 100644 index 0000000..8a117f0 --- /dev/null +++ b/tests/PDMMigrationDedupeTest.php @@ -0,0 +1,450 @@ + */ + public array $folders = []; + /** @var array */ + public array $files = []; + /** @var string[] */ + public array $queries = []; + /** @var array */ + public array $updates = []; + /** @var array */ + public array $deletes = []; + public bool $hasParentKeyColumn = false; + /** @var array */ + public array $indexes = []; + + public function prepare(string $query, ...$args): array + { + return ['query' => $query, 'args' => $args]; + } + + public function esc_like(string $text): string + { + return addcslashes($text, '_%\\'); + } + + public function get_results($query): array + { + [$sql, $args] = $this->split($query); + + if (str_starts_with($sql, 'DESCRIBE')) { + return $this->hasParentKeyColumn ? [(object) ['Field' => 'parent_key']] : []; + } + + if (str_contains($sql, 'GROUP BY parent_key, name')) { + return $this->folderGroups(); + } + + if (str_contains($sql, 'WHERE parent_key = %d AND name = %s AND id <> %d')) { + return $this->duplicateFolderRows((int) $args[0], (string) $args[1], (int) $args[2]); + } + + if (str_contains($sql, 'WHERE relative_path = %s OR relative_path LIKE %s')) { + return $this->rowsByPathPrefix($sql, (string) $args[0]); + } + + if (str_contains($sql, 'FROM `wp_mstv_files`')) { + return $this->fileRows(); + } + + if (str_contains($sql, 'FROM `wp_mstv_folders`')) { + return $this->folderRows(); + } + + return []; + } + + public function get_row($query) + { + [$sql, $args] = $this->split($query); + + if (str_contains($sql, 'WHERE id = %d')) { + $id = (int) $args[0]; + $row = $this->folders[$id] ?? $this->files[$id] ?? null; + + return $row === null + ? null + : (object) ['id' => $id, 'relative_path' => (string) $row->relative_path]; + } + + return null; + } + + public function get_var($query) + { + [$sql, $args] = $this->split($query); + + if (str_contains($sql, 'information_schema.statistics')) { + return isset($this->indexes[(string) $args[1]]) ? 1 : 0; + } + + if (str_contains($sql, 'LEFT(relative_path, 191)')) { + return 0; + } + + return 0; + } + + public function query($query) + { + [$sql, $args] = $this->split($query); + $this->queries[] = $sql; + + if (str_contains($sql, 'ADD COLUMN parent_key')) { + $this->hasParentKeyColumn = true; + + return 1; + } + + if (preg_match('/ADD UNIQUE KEY `([^`]+)`/', $sql, $matches) === 1) { + $this->indexes[$matches[1]] = true; + + return 1; + } + + if (str_starts_with($sql, 'UPDATE `wp_mstv_folders` SET parent_key')) { + foreach ($this->folders as $row) { + $row->parent_key = (int) ($row->parent_id ?? 0); + } + + return count($this->folders); + } + + return 1; + } + + public function update($table, $data, $where, $format = null, $whereFormat = null): int + { + $this->updates[] = ['table' => $table, 'data' => $data, 'where' => $where]; + + return $this->apply($table, $data, $where); + } + + public function delete($table, $where, $format = null): int + { + $this->deletes[] = ['table' => $table, 'where' => $where]; + + $isFolders = str_contains($table, 'folders'); + $removed = 0; + + foreach ($this->rowsFor($isFolders) as $id => $row) { + if (!$this->matchesWhere($row, $where)) { + continue; + } + + if ($isFolders) { + unset($this->folders[$id]); + } else { + unset($this->files[$id]); + } + + $removed++; + } + + return $removed; + } + + private function apply(string $table, array $data, array $where): int + { + $changed = 0; + + foreach ($this->rowsFor(str_contains($table, 'folders')) as $row) { + if (!$this->matchesWhere($row, $where)) { + continue; + } + + foreach ($data as $key => $value) { + $row->{$key} = $value; + } + + $changed++; + } + + return $changed; + } + + /** + * @return array + */ + private function rowsFor(bool $isFolders): array + { + return $isFolders ? $this->folders : $this->files; + } + + private function matchesWhere(object $row, array $where): bool + { + foreach ($where as $key => $value) { + $current = $row->{$key} ?? null; + + if ($value === null) { + if ($current !== null) { + return false; + } + + continue; + } + + if ((string) $current !== (string) $value) { + return false; + } + } + + return true; + } + + private function folderGroups(): array + { + $groups = []; + + foreach ($this->folders as $row) { + $groups[((int) $row->parent_key) . '|' . mb_strtolower((string) $row->name)][] = $row; + } + + $result = []; + + foreach ($groups as $rows) { + if (count($rows) < 2) { + continue; + } + + $ids = array_map(static fn($row): int => (int) $row->id, $rows); + $result[] = (object) [ + 'parent_key' => (int) $rows[0]->parent_key, + 'name' => (string) $rows[0]->name, + 'keep_id' => min($ids), + 'total' => count($rows), + ]; + } + + return $result; + } + + private function duplicateFolderRows(int $parentKey, string $name, int $keepId): array + { + $rows = []; + + foreach ($this->folders as $row) { + if ((int) $row->parent_key === $parentKey + && mb_strtolower((string) $row->name) === mb_strtolower($name) + && (int) $row->id !== $keepId + ) { + $rows[] = (object) ['id' => (int) $row->id, 'relative_path' => (string) $row->relative_path]; + } + } + + usort($rows, static fn($a, $b): int => $a->id <=> $b->id); + + return $rows; + } + + private function rowsByPathPrefix(string $sql, string $prefix): array + { + $isFolders = str_contains($sql, 'FROM `wp_mstv_folders`'); + $rows = []; + + foreach ($this->rowsFor($isFolders) as $row) { + $path = (string) $row->relative_path; + + if ($path === $prefix || str_starts_with($path, $prefix . '/')) { + $rows[] = (object) ['id' => (int) $row->id, 'relative_path' => $path]; + } + } + + return $rows; + } + + private function folderRows(): array + { + $rows = []; + + foreach ($this->folders as $row) { + $rows[] = (object) ['id' => (int) $row->id, 'relative_path' => (string) $row->relative_path]; + } + + return $rows; + } + + private function fileRows(): array + { + $rows = []; + + foreach ($this->files as $row) { + $rows[] = (object) ['id' => (int) $row->id, 'relative_path' => (string) $row->relative_path]; + } + + return $rows; + } + + /** + * @return array{0: string, 1: array} + */ + private function split($query): array + { + if (is_array($query)) { + return [(string) $query['query'], $query['args']]; + } + + return [(string) $query, []]; + } +} + +final class PDMMigrationDedupeTest extends TestCase +{ + protected function setUp(): void + { + $GLOBALS['pdm_test_options'] = []; + } + + public function test_uniqueness_migration_deduplicates_rows_and_adds_indexes(): void + { + $wpdb = $this->buildWpdb(); + + $this->runMigration($wpdb); + + // Duplicate siblings merged onto the lowest id. + self::assertArrayHasKey(1, $wpdb->folders); + self::assertArrayNotHasKey(2, $wpdb->folders); + self::assertArrayHasKey(3, $wpdb->folders); + self::assertArrayNotHasKey(4, $wpdb->folders); + self::assertSame(1, (int) $wpdb->folders[3]->parent_id); + self::assertSame(1, (int) $wpdb->folders[3]->parent_key); + + // Files re-pointed and duplicate file rows removed. + self::assertSame(1, (int) $wpdb->files[10]->folder_id); + self::assertSame(3, (int) $wpdb->files[12]->folder_id); + self::assertArrayNotHasKey(11, $wpdb->files); + + // Explicit ALTERs instead of relying on dbDelta for the unique keys. + self::assertContains( + 'ALTER TABLE `wp_mstv_folders` ADD COLUMN parent_key BIGINT(20) UNSIGNED NOT NULL DEFAULT 0', + $wpdb->queries + ); + self::assertNotEmpty(array_filter( + $wpdb->queries, + static fn(string $sql): bool => str_contains($sql, 'ADD UNIQUE KEY `uq_mstv_folder_parent_name`') + )); + self::assertNotEmpty(array_filter( + $wpdb->queries, + static fn(string $sql): bool => str_contains($sql, 'ADD UNIQUE KEY `uq_mstv_file_relative_path`') + )); + + // Per-folder rules of the duplicate folder are merged onto the survivor. + self::assertNotEmpty(array_filter( + $wpdb->queries, + static fn(string $sql): bool => str_contains( + $sql, + 'UPDATE IGNORE `wp_mstv_folder_permissions` SET folder_id = 1 WHERE folder_id = 2' + ) + )); + self::assertNotEmpty(array_filter( + $wpdb->queries, + static fn(string $sql): bool => str_contains( + $sql, + 'DELETE FROM `wp_mstv_folder_permissions` WHERE folder_id = 2' + ) + )); + + self::assertTrue(MSTV_Activator::schema_migration_applied('folder_parent_key')); + self::assertTrue(MSTV_Activator::schema_migration_applied('folder_unique')); + self::assertTrue(MSTV_Activator::schema_migration_applied('file_unique')); + } + + public function test_uniqueness_migration_is_skipped_once_applied(): void + { + $wpdb = $this->buildWpdb(); + $this->runMigration($wpdb); + + $queriesBefore = count($wpdb->queries); + $updatesBefore = count($wpdb->updates); + $deletesBefore = count($wpdb->deletes); + + $this->runMigration($wpdb); + + self::assertSame($queriesBefore, count($wpdb->queries)); + self::assertSame($updatesBefore, count($wpdb->updates)); + self::assertSame($deletesBefore, count($wpdb->deletes)); + } + + public function test_duplicate_subtree_paths_are_rewritten_onto_the_surviving_path(): void + { + $wpdb = new FakeMigrationWpdb(); + $wpdb->hasParentKeyColumn = true; + $wpdb->folders = [ + 1 => (object) ['id' => 1, 'parent_id' => null, 'parent_key' => 0, 'name' => 'Docs', 'relative_path' => 'Docs'], + 2 => (object) ['id' => 2, 'parent_id' => null, 'parent_key' => 0, 'name' => 'docs', 'relative_path' => 'docs'], + 3 => (object) ['id' => 3, 'parent_id' => 2, 'parent_key' => 2, 'name' => 'Inner', 'relative_path' => 'docs/inner'], + ]; + $wpdb->files = [ + 10 => (object) ['id' => 10, 'folder_id' => 2, 'relative_path' => 'docs/inner/a.pdf'], + ]; + + $this->runMigration($wpdb); + + self::assertArrayNotHasKey(2, $wpdb->folders); + self::assertSame('Docs/inner', (string) $wpdb->folders[3]->relative_path); + self::assertSame(1, (int) $wpdb->folders[3]->parent_id); + self::assertSame('Docs/inner/a.pdf', (string) $wpdb->files[10]->relative_path); + self::assertSame(1, (int) $wpdb->files[10]->folder_id); + } + + public function test_replace_path_prefix_rewrites_only_the_subtree(): void + { + self::assertSame('docs/inner/a.pdf', MSTV_Activator::replace_path_prefix('docs/inner/a.pdf', 'docs', 'docs')); + self::assertSame('archive/a.pdf', MSTV_Activator::replace_path_prefix('docs/a.pdf', 'docs', 'archive')); + self::assertSame('archive', MSTV_Activator::replace_path_prefix('docs', 'docs', 'archive')); + self::assertSame('docs-other/a.pdf', MSTV_Activator::replace_path_prefix('docs-other/a.pdf', 'docs', 'archive')); + self::assertSame('other/docs/a.pdf', MSTV_Activator::replace_path_prefix('other/docs/a.pdf', 'docs', 'archive')); + } + + public function test_duplicate_file_groups_keep_the_lowest_id(): void + { + $groups = MSTV_Activator::duplicate_file_groups([ + (object) ['id' => 5, 'relative_path' => 'a.pdf'], + (object) ['id' => 2, 'relative_path' => 'a.pdf'], + (object) ['id' => 9, 'relative_path' => 'b.pdf'], + ]); + + self::assertCount(1, $groups); + self::assertSame(2, $groups[0]['keep_id']); + self::assertSame([5], $groups[0]['duplicate_ids']); + } + + private function runMigration(FakeMigrationWpdb $wpdb): void + { + $GLOBALS['wpdb'] = $wpdb; + + $method = new ReflectionMethod(MSTV_Activator::class, 'ensure_unique_indexes'); + $method->setAccessible(true); + $method->invoke(null, 'wp_mstv_folders', 'wp_mstv_files', 'wp_mstv_folder_permissions'); + } + + private function buildWpdb(): FakeMigrationWpdb + { + $wpdb = new FakeMigrationWpdb(); + $wpdb->folders = [ + 1 => (object) ['id' => 1, 'parent_id' => null, 'parent_key' => 0, 'name' => 'Docs', 'relative_path' => 'docs'], + 2 => (object) ['id' => 2, 'parent_id' => null, 'parent_key' => 0, 'name' => 'Docs', 'relative_path' => 'docs'], + 3 => (object) ['id' => 3, 'parent_id' => 2, 'parent_key' => 2, 'name' => 'Inner', 'relative_path' => 'docs/inner'], + 4 => (object) ['id' => 4, 'parent_id' => 2, 'parent_key' => 2, 'name' => 'Inner', 'relative_path' => 'docs/inner'], + ]; + $wpdb->files = [ + 10 => (object) ['id' => 10, 'folder_id' => 2, 'relative_path' => 'docs/inner/a.pdf'], + 11 => (object) ['id' => 11, 'folder_id' => 2, 'relative_path' => 'docs/inner/a.pdf'], + 12 => (object) ['id' => 12, 'folder_id' => 4, 'relative_path' => 'docs/inner/b.pdf'], + ]; + + return $wpdb; + } +} diff --git a/tests/PDMReindexLockTest.php b/tests/PDMReindexLockTest.php new file mode 100644 index 0000000..06aeea2 --- /dev/null +++ b/tests/PDMReindexLockTest.php @@ -0,0 +1,55 @@ +acquire()); + self::assertFalse($second->acquire()); + self::assertSame('mstv_auto_reindex_claim_1', $first->option_name()); + self::assertNotFalse(get_option('mstv_auto_reindex_claim_1')); + } + + public function test_released_claim_can_be_acquired_again(): void + { + $lock = new MSTV_Reindex_Lock(1); + + self::assertTrue($lock->acquire()); + $lock->release(); + + self::assertFalse(get_option('mstv_auto_reindex_claim_1')); + self::assertTrue((new MSTV_Reindex_Lock(1))->acquire()); + } + + public function test_fresh_claim_blocks_concurrent_requests(): void + { + $GLOBALS['pdm_test_options']['mstv_auto_reindex_claim_1'] = (string) time(); + + self::assertFalse((new MSTV_Reindex_Lock(1))->acquire()); + } + + public function test_stale_claim_is_taken_over(): void + { + $GLOBALS['pdm_test_options']['mstv_auto_reindex_claim_1'] = (string) (time() - MSTV_Reindex_Lock::TTL - 1); + + self::assertTrue((new MSTV_Reindex_Lock(1))->acquire()); + } + + public function test_claims_are_scoped_per_site(): void + { + self::assertTrue((new MSTV_Reindex_Lock(1))->acquire()); + self::assertTrue((new MSTV_Reindex_Lock(2))->acquire()); + } +} diff --git a/tests/PDMRestControllerTest.php b/tests/PDMRestControllerTest.php index 18c522a..4183702 100644 --- a/tests/PDMRestControllerTest.php +++ b/tests/PDMRestControllerTest.php @@ -624,7 +624,8 @@ public function test_browser_allows_reindex_retry_after_metadata_failure(): void $controller->get_browser_data(new WP_REST_Request()); - self::assertFalse(get_transient('mstv_auto_reindex_1')); + // A failed scan releases the claim so the next request can retry. + self::assertFalse(get_option('mstv_auto_reindex_claim_1')); } public function test_create_folder_is_forbidden_without_manage_permission(): void diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 638a82d..b95c7cc 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -364,8 +364,14 @@ function update_option($key, $value) return true; } -function add_option($key, $value) +function add_option($key, $value, $deprecated = '', $autoload = 'yes') { + if (array_key_exists($key, $GLOBALS['pdm_test_options'])) { + // WordPress add_option() fails on an existing option; callers rely on that + // for atomic claims. + return false; + } + $GLOBALS['pdm_test_options'][$key] = $value; return true; @@ -745,6 +751,7 @@ public function capabilities(): array require_once __DIR__ . '/../includes/class-mstv-repository-groups.php'; require_once __DIR__ . '/../includes/class-mstv-repository-permissions.php'; require_once __DIR__ . '/../includes/class-mstv-quota.php'; +require_once __DIR__ . '/../includes/class-mstv-reindex-lock.php'; require_once __DIR__ . '/../includes/class-mstv-hooks.php'; require_once __DIR__ . '/../includes/class-mstv-notifications.php'; require_once __DIR__ . '/../includes/trait-mstv-binary-stream.php'; From e50e1e2c1706502dd01d0427c37dd86955191af7 Mon Sep 17 00:00:00 2001 From: Michael Gasperini <163041857+TheStreamCode@users.noreply.github.com> Date: Wed, 21 Oct 2026 21:57:22 +0200 Subject: [PATCH 2/4] fix(storage): fail closed when the private storage is unprotected Refuse uploads when the storage directory is not writable or when the required protection files (.htaccess, web.config, index.php) cannot be created, and report the failing files in the error. Replace the dismissible storage warning with a persistent, non-dismissible admin notice for the blocking states and for Nginx, which ignores .htaccess. Document the out-of-webroot storage path as the recommended deployment in the README and the maintainer notes. --- README.md | 12 +- docs/maintainer/local-development.md | 2 +- includes/class-mstv-admin.php | 60 +++++++-- includes/class-mstv-helpers.php | 162 ++++++++++++++++++------- includes/class-mstv-storage.php | 91 ++++++++++++-- tests/PDMAdminSecurityNoticeTest.php | 151 ++++++++++++++++++----- tests/PDMStorageProtectionTest.php | 174 +++++++++++++++++++++++++++ 7 files changed, 552 insertions(+), 100 deletions(-) create mode 100644 tests/PDMStorageProtectionTest.php diff --git a/README.md b/README.md index 9282f78..b350008 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,7 @@ Open `TeamVault > Settings` after activation. The main settings are: Groups, per-folder permissions, quotas, reports, and notifications have dedicated admin pages. Configure access before inviting non-administrator users. -Runtime configuration is stored in WordPress options; TeamVault does not read settings from environment variables or a repository `.env` file. Keep database credentials and salts in the site-level `wp-config.php`. +Runtime configuration is stored in WordPress options; TeamVault reads no settings from environment variables or a repository `.env` file. Keep database credentials and salts in the site-level `wp-config.php`. ## Privacy and security @@ -86,8 +86,8 @@ Private files are never served through public media URLs: downloads and previews - Optional whitelist mode adds a second authorization layer for selected users. - Per-folder permissions limit access to the granted users, groups, and actions, with inheritance from parent folders. Folders without rules keep the capability-based behavior, and Administrators always keep full access. - Settings, groups, quotas, notifications, reports, logs, whitelist management, maintenance tools, and uninstall data controls require `manage_options`. -- Storage defaults to `wp-content/uploads/private-documents/` and is protected with server-level deny files: the generated `.htaccess` on Apache and LiteSpeed, `web.config` on IIS, and an equivalent Nginx rule that denies direct requests to that path. -- For high-sensitivity deployments, prefer a custom storage path outside the public webroot. The storage widget shows only TeamVault usage. +- Storage defaults to `wp-content/uploads/private-documents/`, protected with server-level deny files: the generated `.htaccess` on Apache and LiteSpeed, `web.config` on IIS, and an equivalent Nginx rule that denies direct requests. Uploads are refused when these files cannot be created. +- Recommended: a custom storage path outside the public webroot, especially on Nginx, which ignores `.htaccess`. The storage widget shows only TeamVault usage. See [`SECURITY.md`](SECURITY.md) for vulnerability reporting. @@ -103,11 +103,11 @@ Whitelist mode is an allowlist. Keep the current administrator account in the al ### Files are missing after a site migration -A site migrated without the private storage folder keeps TeamVault records in the database while the original binaries are missing. Copy the storage directory during migrations, then use the cleanup and reindex tools in the settings screen after confirming the copy. +A site migrated without the private storage folder keeps TeamVault records in the database while the original binaries are missing. Copy the storage directory during migrations, then use the cleanup and reindex tools after confirming the copy. ### Uploads are rejected -Check the allowed extensions, the maximum file size, and the PHP upload limits. TeamVault validates the extension, detected MIME type, size, and dangerous content patterns before storing a file, and reports the detected content type on a mismatch. +Check the allowed extensions, the maximum file size, and the PHP upload limits. TeamVault validates extension, detected MIME type, size, and dangerous content patterns before storing a file, and reports the detected content type on a mismatch. ## Development @@ -134,7 +134,7 @@ node --check assets/js/admin-notice-dismiss.js node --test tests/plugin-check-output.test.mjs ``` -There is no asset compilation step: runtime PHP, CSS, and JavaScript are committed as source. `git archive --format=zip --prefix=mikesoft-teamvault/ --output=mikesoft-teamvault.zip HEAD` builds a package from a committed revision using the `.gitattributes` export excludes. +Runtime PHP, CSS, and JavaScript are committed as source. `git archive --format=zip --prefix=mikesoft-teamvault/ --output=mikesoft-teamvault.zip HEAD` builds a package from a committed revision using the `.gitattributes` export excludes. Maintainer and developer notes: diff --git a/docs/maintainer/local-development.md b/docs/maintainer/local-development.md index 13c5fc3..58dc900 100644 --- a/docs/maintainer/local-development.md +++ b/docs/maintainer/local-development.md @@ -137,7 +137,7 @@ TeamVault uses WordPress options for runtime settings and does not require a rep Do not store WordPress credentials, salts, Composer authentication, deployment credentials, database exports, or production documents in this repository. `.env*` and `auth.json` are ignored and excluded from release packages; use local host configuration, CI secrets, or the operating-system credential store instead. -An advanced custom storage root is operator-provisioned rather than entered in the settings UI. It must be writable by WordPress, remain inside the intended deployment boundary, and be protected from direct web access. +An advanced custom storage root is operator-provisioned rather than entered in the settings UI. It must be writable by WordPress, remain inside the intended deployment boundary, and be protected from direct web access. The recommended deployment is a storage root outside the public webroot, especially on Nginx, which ignores the generated `.htaccess`. Uploads are refused while the required protection files (`.htaccess`, `web.config`, `index.php`) cannot be created in the storage directory. ## Manual QA Focus diff --git a/includes/class-mstv-admin.php b/includes/class-mstv-admin.php index a6dce53..93cdcd5 100644 --- a/includes/class-mstv-admin.php +++ b/includes/class-mstv-admin.php @@ -51,24 +51,61 @@ public function add_plugin_row_meta($links, $file): array return $links; } + /** + * Persistent storage-protection notice. + * + * The notice is intentionally not dismissible: it reports a blocking state + * (uploads refused because the directory is not writable or the protection + * files are missing) or an unverifiable one (Nginx ignores .htaccess), and + * both need an operator decision before private documents are safe. + */ public function render_storage_security_notice(): void { - if (!$this->is_teamvault_settings_page() || !$this->current_user_can_admin() || !$this->settings->is_storage_path_inside_uploads()) { + if (!$this->is_teamvault_admin_page() || !$this->current_user_can_admin()) { return; } - if (get_user_meta(get_current_user_id(), 'mstv_notice_storage_dismissed', true)) { + $status = (new MSTV_Storage($this->settings))->get_storage_protection_status(); + + $blocking = !$status['directory_writable'] || !$status['ok']; + $nginxWarning = !$blocking + && $status['inside_uploads'] + && $this->is_nginx_server((string) $status['server_software']); + + if (!$blocking && !$nginxWarning) { return; } - echo '

'; - echo esc_html__( - 'TeamVault stores private files under WordPress uploads. Confirm your web server blocks direct access to the private-documents directory, especially on Nginx.', - 'mikesoft-teamvault' - ); + $messages = []; + + if (!$status['directory_writable']) { + $messages[] = __('TeamVault cannot write to the private storage directory, so uploads are refused. Make the directory writable by WordPress, then reload this page.', 'mikesoft-teamvault'); + } + + if (!$status['ok']) { + $messages[] = sprintf( + /* translators: %s: comma-separated list of protection files that could not be created. */ + __('TeamVault could not create the protection files (%s) that block direct web access to the private storage directory. Uploads are refused until the directory is writable and the files exist.', 'mikesoft-teamvault'), + implode(', ', $status['failed']) + ); + } + + if ($nginxWarning) { + $messages[] = __('This server runs Nginx, which ignores .htaccess. Confirm that a server rule denies direct requests to the private storage directory, or set a custom storage path outside the public webroot (recommended).', 'mikesoft-teamvault'); + } + + $noticeClass = $blocking ? 'notice notice-error' : 'notice notice-warning'; + + echo '

'; + echo esc_html(implode(' ', $messages)); echo '

'; } + /** + * Legacy dismissal endpoint. The storage protection notice is persistent, so + * the stored preference no longer hides it; the endpoint stays for + * compatibility with admin scripts already loaded in the browser. + */ public function handle_dismiss_storage_notice(): void { if (!$this->current_user_can_admin()) { @@ -80,12 +117,17 @@ public function handle_dismiss_storage_notice(): void wp_die(); } - private function is_teamvault_settings_page(): bool + private function is_nginx_server(string $serverSoftware): bool + { + return stripos($serverSoftware, 'nginx') !== false; + } + + private function is_teamvault_admin_page(): bool { // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Read-only admin page check used only to decide whether to render a notice. $page = isset($_GET['page']) ? sanitize_text_field(wp_unslash($_GET['page'])) : ''; - return $page === 'mikesoft-teamvault-settings'; + return in_array($page, ['mikesoft-teamvault', 'mikesoft-teamvault-settings'], true); } public function add_menu(): void diff --git a/includes/class-mstv-helpers.php b/includes/class-mstv-helpers.php index 92852df..0378c2c 100644 --- a/includes/class-mstv-helpers.php +++ b/includes/class-mstv-helpers.php @@ -223,55 +223,129 @@ public static function send_json_success(array $data = [], int $statusCode = 200 wp_send_json_success($data, $statusCode); } - public static function create_protection_files(string $path): void + /** + * Names and contents of the files that protect the private storage directory. + * + * The first three entries are required for the directory to be considered + * protected; the storage marker is informational and never blocks uploads. + * + * @return array + */ + public static function protection_file_contents(): array { - $htaccess = $path . '/.htaccess'; - if (!file_exists($htaccess)) { - // Cover Apache 2.4 (mod_authz_core), Apache 2.2 (mod_access_compat) and, - // as a last resort, mod_rewrite. On Apache 2.4 the legacy "Order/Deny" - // directives are inert unless mod_access_compat is loaded, so the native - // "Require all denied" must be emitted too. - $content = "# Mikesoft TeamVault - Access Denied\n"; - $content .= "\n"; - $content .= " Require all denied\n"; - $content .= "\n"; - $content .= "\n"; - $content .= " Order deny,allow\n"; - $content .= " Deny from all\n"; - $content .= "\n"; - $content .= "\n"; - $content .= " RewriteEngine On\n"; - $content .= " RewriteRule .* - [F]\n"; - $content .= "\n"; - @file_put_contents($htaccess, $content); - } + // Cover Apache 2.4 (mod_authz_core), Apache 2.2 (mod_access_compat) and, + // as a last resort, mod_rewrite. On Apache 2.4 the legacy "Order/Deny" + // directives are inert unless mod_access_compat is loaded, so the native + // "Require all denied" must be emitted too. + $htaccess = "# Mikesoft TeamVault - Access Denied\n"; + $htaccess .= "\n"; + $htaccess .= " Require all denied\n"; + $htaccess .= "\n"; + $htaccess .= "\n"; + $htaccess .= " Order deny,allow\n"; + $htaccess .= " Deny from all\n"; + $htaccess .= "\n"; + $htaccess .= "\n"; + $htaccess .= " RewriteEngine On\n"; + $htaccess .= " RewriteRule .* - [F]\n"; + $htaccess .= "\n"; + + $webconfig = "\n"; + $webconfig .= "\n"; + $webconfig .= " \n"; + $webconfig .= " \n"; + $webconfig .= " \n"; + $webconfig .= " \n"; + $webconfig .= " \n"; + $webconfig .= " \n"; + $webconfig .= " \n"; + $webconfig .= " \n"; + $webconfig .= " \n"; + $webconfig .= " \n"; + $webconfig .= ""; + + return [ + '.htaccess' => $htaccess, + 'web.config' => $webconfig, + 'index.php' => " "Mikesoft TeamVault storage marker\n", + ]; + } - $webconfig = $path . '/web.config'; - if (!file_exists($webconfig)) { - $content = "\n"; - $content .= "\n"; - $content .= " \n"; - $content .= " \n"; - $content .= " \n"; - $content .= " \n"; - $content .= " \n"; - $content .= " \n"; - $content .= " \n"; - $content .= " \n"; - $content .= " \n"; - $content .= " \n"; - $content .= ""; - @file_put_contents($webconfig, $content); - } + /** + * Create the web-server protection files in a storage directory. + * + * Fail-closed contract: the result reports every file that could not be + * created so callers can refuse uploads instead of serving private documents + * from an unprotected directory. An existing non-empty file counts as + * protection even when the directory is not writable any more; an empty or + * missing file is rewritten and reported as failed when the write fails. + * + * @param string $path Absolute storage directory. + * @param callable|null $writer Optional writer receiving (string $file, string $content): bool, + * used by tests to simulate a failing filesystem. + * @return array{files: array, failed: string[], marker: bool, ok: bool} + */ + public static function ensure_protection_files(string $path, ?callable $writer = null): array + { + $path = rtrim($path, '/\\'); + $results = [ + 'files' => [], + 'failed' => [], + 'marker' => true, + 'ok' => true, + ]; - $index = $path . '/index.php'; - if (!file_exists($index)) { - @file_put_contents($index, " $content) { + $target = $path . '/' . $name; + $isMarker = $name === '.mstv-storage'; + $exists = file_exists($target); + + if ($exists && ($isMarker || $name === 'index.php' || filesize($target) > 0)) { + $results['files'][$name] = true; + continue; + } + + $written = $writer !== null + ? (bool) call_user_func($writer, $target, $content) + : self::write_protection_file($target, $content); + + $results['files'][$name] = $written; + + if (!$written) { + if ($isMarker) { + $results['marker'] = false; + } else { + $results['failed'][] = $name; + } + } } + + $results['ok'] = $results['failed'] === []; + + return $results; + } + + /** + * Backward-compatible wrapper that only creates the protection files. + */ + public static function create_protection_files(string $path): void + { + self::ensure_protection_files($path); + } + + private static function write_protection_file(string $target, string $content): bool + { + // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_file_put_contents -- Protection files live at the storage root and must be written even when WP_Filesystem is not initialized. + return @file_put_contents($target, $content) !== false; } } diff --git a/includes/class-mstv-storage.php b/includes/class-mstv-storage.php index 7cbe056..0334e3a 100644 --- a/includes/class-mstv-storage.php +++ b/includes/class-mstv-storage.php @@ -9,10 +9,14 @@ class MSTV_Storage private MSTV_Settings $settings; private MSTV_Filesystem $filesystem; - public function __construct(MSTV_Settings $settings) + /** @var callable|null Optional writer used to create the storage protection files. */ + private $protectionWriter; + + public function __construct(MSTV_Settings $settings, ?callable $protectionWriter = null) { $this->settings = $settings; $this->filesystem = new MSTV_Filesystem($settings->get_storage_path()); + $this->protectionWriter = $protectionWriter; } public function get_filesystem(): MSTV_Filesystem @@ -158,6 +162,24 @@ public function store_uploaded_file( ]; } + $protection = $this->get_storage_protection_status(); + + if (!$protection['ok'] || !$protection['directory_writable']) { + // Fail closed: without the web-server protection files the directory may + // be reachable by exact URL, so refuse to add another private document. + return [ + 'success' => false, + 'error_code' => 'storage_unprotected', + 'error' => sprintf( + /* translators: %s: comma-separated list of protection files that could not be created. */ + __('Upload refused: TeamVault could not create the protection files (%s) in the private storage directory. Make the directory writable, or use a storage path outside the public webroot, then retry.', 'mikesoft-teamvault'), + $protection['failed'] === [] + ? __('directory not writable', 'mikesoft-teamvault') + : implode(', ', $protection['failed']) + ), + ]; + } + $extension = strtolower(pathinfo($uploadedFile['name'], PATHINFO_EXTENSION)); $storedName = MSTV_Helpers::generate_secure_filename($extension); @@ -528,14 +550,48 @@ public function ensure_storage_directory(): bool return false; } - $this->create_protection_files($basePath); + MSTV_Helpers::ensure_protection_files($basePath, $this->protectionWriter); return $this->filesystem->is_writable($basePath); } - private function create_protection_files(string $path): void + /** + * Report whether the private storage directory is writable and protected. + * + * The protection files are created here when they are missing, so the result + * reflects the live state instead of a cached assumption. Callers that write + * private documents must refuse the operation when the result is not safe. + * + * @return array{ + * directory_writable: bool, + * files: array, + * failed: string[], + * marker: bool, + * ok: bool, + * inside_uploads: bool, + * server_software: string + * } + */ + public function get_storage_protection_status(): array { - MSTV_Helpers::create_protection_files($path); + $basePath = $this->get_base_path(); + $directoryWritable = $basePath !== '' && is_dir($basePath) && $this->filesystem->is_writable($basePath); + $protection = MSTV_Helpers::ensure_protection_files($basePath, $this->protectionWriter); + + $serverSoftware = ''; + if (isset($_SERVER['SERVER_SOFTWARE']) && is_string($_SERVER['SERVER_SOFTWARE'])) { + $serverSoftware = sanitize_text_field(wp_unslash($_SERVER['SERVER_SOFTWARE'])); + } + + return [ + 'directory_writable' => $directoryWritable, + 'files' => $protection['files'], + 'failed' => $protection['failed'], + 'marker' => $protection['marker'], + 'ok' => $protection['ok'], + 'inside_uploads' => $this->settings->is_storage_path_inside_uploads(), + 'server_software' => $serverSoftware, + ]; } private function reindex_directory( @@ -568,11 +624,20 @@ private function reindex_directory( ]); if ($folderId <= 0) { - return false; + $existingFolder = $folderRepo->find_by_relative_path($itemRelativePath); + + if ($existingFolder === null) { + return false; + } + + // A concurrent reindex created the same folder first: reuse + // the surviving row instead of failing the whole scan. + $folderId = (int) $existingFolder->id; + $folderMap[$itemRelativePath] = $folderId; + } else { + $folderMap[$itemRelativePath] = $folderId; + $stats['folders_created']++; } - - $folderMap[$itemRelativePath] = $folderId; - $stats['folders_created']++; } if (!$this->reindex_directory($itemRelativePath, $folderRepo, $filesRepo, $folderMap, $fileMap, $createdBy, $stats)) { @@ -609,7 +674,15 @@ private function reindex_directory( ]); if ($fileId <= 0) { - return false; + if ($filesRepo->find_by_relative_path($itemRelativePath) === null) { + return false; + } + + // A concurrent reindex created the same file path first: keep the + // surviving row and continue the scan. + $fileMap[$itemRelativePath] = true; + $stats['files_skipped']++; + continue; } $fileMap[$itemRelativePath] = true; diff --git a/tests/PDMAdminSecurityNoticeTest.php b/tests/PDMAdminSecurityNoticeTest.php index d2c93cd..00f7f4a 100644 --- a/tests/PDMAdminSecurityNoticeTest.php +++ b/tests/PDMAdminSecurityNoticeTest.php @@ -6,69 +6,158 @@ final class PDMAdminSecurityNoticeTest extends TestCase { + private string $uploadBase; + private string $insideUploadsPath; + private string $outsideUploadsPath; + protected function setUp(): void { + $this->uploadBase = sys_get_temp_dir() . '/mstv-test-uploads'; + $this->insideUploadsPath = $this->uploadBase . '/private-documents'; + $this->outsideUploadsPath = sys_get_temp_dir() . '/mstv-notice-test-' . bin2hex(random_bytes(6)); + $GLOBALS['pdm_test_is_user_logged_in'] = true; - $GLOBALS['pdm_test_current_user_caps'] = [ - 'manage_options' => true, - ]; - $GLOBALS['pdm_test_options'] = [ - 'mstv_storage_path' => sys_get_temp_dir() . '/private-documents', - ]; + $GLOBALS['pdm_test_current_user_caps'] = ['manage_options' => true]; + $GLOBALS['pdm_test_options'] = []; + $GLOBALS['pdm_test_user_meta'] = []; + $_GET['page'] = 'mikesoft-teamvault-settings'; + unset($_SERVER['SERVER_SOFTWARE']); + + $GLOBALS['wp_filesystem'] = new class { + public function is_writable($path): bool + { + return is_writable($path); + } + }; } protected function tearDown(): void { + unset($GLOBALS['wp_filesystem'], $_GET['page'], $_SERVER['SERVER_SOFTWARE']); $GLOBALS['pdm_test_is_user_logged_in'] = false; $GLOBALS['pdm_test_current_user_caps'] = []; $GLOBALS['pdm_test_options'] = []; - $GLOBALS['pdm_test_current_user_id'] = 0; $GLOBALS['pdm_test_user_meta'] = []; - unset($_GET['page']); + + $this->deleteDirectory($this->insideUploadsPath); + $this->deleteDirectory($this->outsideUploadsPath); } - public function test_storage_security_notice_is_rendered_for_uploads_storage_path_on_settings_page(): void + public function test_blocking_notice_is_rendered_when_storage_is_unprotected(): void { - $_GET['page'] = 'mikesoft-teamvault-settings'; - $admin = new MSTV_Admin(new MSTV_Settings()); + // No storage path configured: the default uploads directory does not exist, + // so the protection files cannot be created. + $notice = $this->renderNotice(); - ob_start(); - $admin->render_storage_security_notice(); - $notice = (string) ob_get_clean(); + self::assertStringContainsString('notice-error', $notice); + self::assertStringContainsString('mstv-storage-security-notice', $notice); + self::assertStringNotContainsString('is-dismissible', $notice); + self::assertStringContainsString('Uploads are refused', $notice); + } + + public function test_notice_is_not_dismissible_and_survives_a_previous_dismissal(): void + { + $GLOBALS['pdm_test_current_user_id'] = 1; + $GLOBALS['pdm_test_user_meta'] = [ + 1 => ['mstv_notice_storage_dismissed' => '1'], + ]; + + $notice = $this->renderNotice(); - self::assertStringContainsString('notice-warning', $notice); - self::assertStringContainsString('is-dismissible', $notice); self::assertStringContainsString('mstv-storage-security-notice', $notice); - self::assertStringContainsString('private files under WordPress uploads', $notice); + self::assertStringNotContainsString('is-dismissible', $notice); + } + + public function test_nginx_warning_is_rendered_for_storage_inside_uploads(): void + { + $this->prepareProtectedStorageInsideUploads(); + $_SERVER['SERVER_SOFTWARE'] = 'nginx/1.24.0'; + + $notice = $this->renderNotice(); + + self::assertStringContainsString('notice-warning', $notice); self::assertStringContainsString('Nginx', $notice); + self::assertStringContainsString('outside the public webroot', $notice); + self::assertStringNotContainsString('is-dismissible', $notice); + } + + public function test_notice_is_suppressed_for_a_protected_out_of_webroot_storage(): void + { + $this->prepareProtectedStorage($this->outsideUploadsPath); + $GLOBALS['pdm_test_options']['mstv_storage_path'] = $this->outsideUploadsPath; + + self::assertSame('', $this->renderNotice()); } - public function test_storage_security_notice_is_suppressed_outside_settings_page(): void + public function test_notice_is_rendered_on_the_file_manager_page(): void { $_GET['page'] = 'mikesoft-teamvault'; + + self::assertStringContainsString('mstv-storage-security-notice', $this->renderNotice()); + } + + public function test_notice_is_suppressed_outside_teamvault_pages(): void + { + $_GET['page'] = 'edit.php'; + + self::assertSame('', $this->renderNotice()); + } + + private function renderNotice(): string + { $admin = new MSTV_Admin(new MSTV_Settings()); ob_start(); $admin->render_storage_security_notice(); - $notice = (string) ob_get_clean(); - self::assertSame('', $notice); + return (string) ob_get_clean(); } - public function test_storage_security_notice_is_suppressed_after_dismiss(): void + private function prepareProtectedStorageInsideUploads(): void { - $_GET['page'] = 'mikesoft-teamvault-settings'; - $GLOBALS['pdm_test_current_user_id'] = 1; - $GLOBALS['pdm_test_user_meta'] = [ - 1 => ['mstv_notice_storage_dismissed' => '1'], - ]; + $this->prepareProtectedStorage($this->insideUploadsPath); + $GLOBALS['pdm_test_options']['mstv_storage_path'] = $this->insideUploadsPath; + } - $admin = new MSTV_Admin(new MSTV_Settings()); + private function prepareProtectedStorage(string $path): void + { + if (!is_dir($path)) { + mkdir($path, 0777, true); + } - ob_start(); - $admin->render_storage_security_notice(); - $notice = (string) ob_get_clean(); + file_put_contents($path . '/.htaccess', str_repeat('x', 10)); + file_put_contents($path . '/web.config', str_repeat('y', 10)); + file_put_contents($path . '/index.php', 'deleteDirectory($itemPath); + continue; + } + + unlink($itemPath); + } - self::assertSame('', $notice); + rmdir($path); } } diff --git a/tests/PDMStorageProtectionTest.php b/tests/PDMStorageProtectionTest.php new file mode 100644 index 0000000..4db338d --- /dev/null +++ b/tests/PDMStorageProtectionTest.php @@ -0,0 +1,174 @@ +storagePath = sys_get_temp_dir() . '/mstv-protection-test-' . bin2hex(random_bytes(6)); + mkdir($this->storagePath, 0777, true); + file_put_contents($this->storagePath . '/.mstv-storage', "marker\n"); + + $GLOBALS['pdm_test_options'] = ['mstv_storage_path' => $this->storagePath]; + $GLOBALS['wp_filesystem'] = new class { + public function is_writable($path): bool + { + return is_writable($path); + } + }; + } + + protected function tearDown(): void + { + unset($GLOBALS['wp_filesystem']); + $this->deleteDirectory($this->storagePath); + $GLOBALS['pdm_test_options'] = []; + } + + public function test_failing_writer_reports_every_required_protection_file(): void + { + $writer = static function (): bool { + return false; + }; + + $result = MSTV_Helpers::ensure_protection_files($this->storagePath, $writer); + + self::assertFalse($result['ok']); + self::assertSame(['.htaccess', 'web.config', 'index.php'], $result['failed']); + self::assertFalse($result['files']['.htaccess']); + self::assertFalse($result['files']['web.config']); + self::assertFalse($result['files']['index.php']); + self::assertFalse($result['marker']); + } + + public function test_existing_protection_files_are_not_rewritten(): void + { + file_put_contents($this->storagePath . '/.htaccess', str_repeat('x', 10)); + file_put_contents($this->storagePath . '/web.config', str_repeat('y', 10)); + file_put_contents($this->storagePath . '/index.php', 'storagePath . '/.mstv-storage', 'marker'); + + $calls = 0; + $writer = static function () use (&$calls): bool { + $calls++; + + return false; + }; + + $result = MSTV_Helpers::ensure_protection_files($this->storagePath, $writer); + + self::assertTrue($result['ok']); + self::assertSame(0, $calls); + } + + public function test_empty_htaccess_is_rewritten_and_fails_closed(): void + { + file_put_contents($this->storagePath . '/.htaccess', ''); + + $writer = static function (string $file): bool { + return !str_ends_with($file, '.htaccess'); + }; + + $result = MSTV_Helpers::ensure_protection_files($this->storagePath, $writer); + + self::assertFalse($result['ok']); + self::assertSame(['.htaccess'], $result['failed']); + } + + public function test_empty_storage_path_is_reported_as_unprotected(): void + { + $result = MSTV_Helpers::ensure_protection_files('', static fn(): bool => true); + + self::assertFalse($result['ok']); + self::assertSame(['.htaccess', 'web.config', 'index.php'], $result['failed']); + self::assertFalse($result['marker']); + } + + public function test_storage_status_reports_an_unprotected_directory(): void + { + $storage = new MSTV_Storage(new MSTV_Settings(), static fn(): bool => false); + $status = $storage->get_storage_protection_status(); + + self::assertFalse($status['ok']); + self::assertTrue($status['directory_writable']); + self::assertSame(['.htaccess', 'web.config', 'index.php'], $status['failed']); + self::assertFalse($status['inside_uploads']); + } + + public function test_upload_is_refused_when_protection_files_cannot_be_written(): void + { + $source = $this->storagePath . '/source.pdf'; + file_put_contents($source, '%PDF-1.4'); + + $folderRepo = $this->getMockBuilder(MSTV_Repository_Folders::class)->disableOriginalConstructor()->getMock(); + $storage = new MSTV_Storage(new MSTV_Settings(), static fn(): bool => false); + + $result = $storage->store_uploaded_file([ + 'name' => 'contract.pdf', + 'tmp_name' => $source, + ], null, $folderRepo); + + self::assertFalse($result['success']); + self::assertSame('storage_unprotected', $result['error_code']); + self::assertStringContainsString('Upload refused', $result['error']); + self::assertStringContainsString('.htaccess', $result['error']); + + $stored = glob($this->storagePath . '/*.pdf') ?: []; + self::assertSame([$source], $stored); + } + + public function test_upload_succeeds_when_protection_files_can_be_written(): void + { + $source = $this->storagePath . '/source.pdf'; + file_put_contents($source, '%PDF-1.4'); + + $folderRepo = $this->getMockBuilder(MSTV_Repository_Folders::class)->disableOriginalConstructor()->getMock(); + $storage = new MSTV_Storage(new MSTV_Settings()); + + $result = $storage->store_uploaded_file([ + 'name' => 'contract.pdf', + 'tmp_name' => $source, + ], null, $folderRepo); + + self::assertTrue($result['success']); + self::assertFileExists($this->storagePath . '/.htaccess'); + self::assertFileExists($this->storagePath . '/web.config'); + self::assertFileExists($this->storagePath . '/index.php'); + self::assertFileExists($this->storagePath . '/' . $result['stored_name']); + } + + private function deleteDirectory(string $path): void + { + if (!is_dir($path)) { + return; + } + + $items = scandir($path); + + if ($items === false) { + return; + } + + foreach ($items as $item) { + if (in_array($item, ['.', '..'], true)) { + continue; + } + + $itemPath = $path . DIRECTORY_SEPARATOR . $item; + + if (is_dir($itemPath)) { + $this->deleteDirectory($itemPath); + continue; + } + + unlink($itemPath); + } + + rmdir($path); + } +} From 5732c2ffd697bf3fdbb29ede5df38fb7bcdbd3bc Mon Sep 17 00:00:00 2001 From: Michael Gasperini <163041857+TheStreamCode@users.noreply.github.com> Date: Wed, 21 Oct 2026 21:58:55 +0200 Subject: [PATCH 3/4] test(db): reset option state between REST controller tests The auto-reindex claim is persisted in an option, so tests that exercise the browser endpoint must start from a clean option state; without it the claim from an earlier test blocks the reindex expectations of the next one. --- tests/PDMRestControllerTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/PDMRestControllerTest.php b/tests/PDMRestControllerTest.php index 4183702..6b10f06 100644 --- a/tests/PDMRestControllerTest.php +++ b/tests/PDMRestControllerTest.php @@ -6,6 +6,14 @@ final class PDMRestControllerTest extends TestCase { + protected function setUp(): void + { + // Each test starts without persisted option state: the auto-reindex claim + // and other options must not leak between tests. + $GLOBALS['pdm_test_options'] = []; + $GLOBALS['pdm_test_transients'] = []; + } + public function test_resolve_folder_id_returns_error_for_missing_folder(): void { $folderRepo = $this->createMock(MSTV_Repository_Folders::class); From 3076b842aa6a760a7ec00393f18938691570289b Mon Sep 17 00:00:00 2001 From: Michael Gasperini <163041857+TheStreamCode@users.noreply.github.com> Date: Wed, 21 Oct 2026 22:06:10 +0200 Subject: [PATCH 4/4] fix: resolve PHPUnit and Plugin Check failures on the uniqueness changes Cover the six new storage protection strings in the four runtime language maps, drop the obsolete dismissible-warning string, and update the POT template. Suppress the WordPress.DB sniffs for the migration region, where validated plugin table identifiers cannot be placeholders, and fix the new tests: stub the folder-name validator, observe the storage marker from a clean directory, and assert the prepared arguments of the permission merge queries. --- includes/class-mstv-activator.php | 4 ++++ includes/class-mstv-i18n.php | 28 ++++++++++++++++++++++++---- languages/mikesoft-teamvault.pot | 24 ++++++++++++++++++++++-- tests/PDMDuplicateCreationTest.php | 3 ++- tests/PDMMigrationDedupeTest.php | 30 ++++++++++++++++++------------ tests/PDMStorageProtectionTest.php | 3 +++ 6 files changed, 73 insertions(+), 19 deletions(-) diff --git a/includes/class-mstv-activator.php b/includes/class-mstv-activator.php index aa3eab9..2d9b82e 100644 --- a/includes/class-mstv-activator.php +++ b/includes/class-mstv-activator.php @@ -198,6 +198,8 @@ private static function create_tables(): void self::ensure_unique_indexes($folders_table, $files_table, $folder_permissions_table); } + // phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.DirectDatabaseQuery.SchemaChange,WordPress.DB.PreparedSQL.NotPrepared,WordPress.DB.PreparedSQL.InterpolatedNotPrepared,PluginCheck.Security.DirectDB.UnescapedDBParameter -- The uniqueness migration operates on validated blog-specific plugin tables and needs explicit ALTER statements; identifiers cannot be placeholders. + /** * Whether a one-time schema migration has been applied. * @@ -669,6 +671,8 @@ private static function log_schema_error(string $context): void error_log('TeamVault: unable to apply the uniqueness migration (' . $context . ').'); } + // phpcs:enable WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.DirectDatabaseQuery.SchemaChange,WordPress.DB.PreparedSQL.NotPrepared,WordPress.DB.PreparedSQL.InterpolatedNotPrepared,PluginCheck.Security.DirectDB.UnescapedDBParameter + private static function normalize_logs_table(string $logsTable): void { global $wpdb; diff --git a/includes/class-mstv-i18n.php b/includes/class-mstv-i18n.php index 60c0341..50599ed 100644 --- a/includes/class-mstv-i18n.php +++ b/includes/class-mstv-i18n.php @@ -226,7 +226,12 @@ class MSTV_I18n 'Previous' => 'Precedente', 'Preview is not available for this file type.' => 'Anteprima non disponibile per questo tipo di file.', 'TeamVault Settings' => 'Impostazioni TeamVault', - 'TeamVault stores private files under WordPress uploads. Confirm your web server blocks direct access to the private-documents directory, especially on Nginx.' => 'TeamVault archivia file privati nella directory uploads di WordPress. Verifica che il server web blocchi l\'accesso diretto alla directory private-documents, soprattutto su Nginx.', + 'A file with this storage path already exists.' => 'Esiste già un file con questo percorso di archiviazione.', + 'TeamVault cannot write to the private storage directory, so uploads are refused. Make the directory writable by WordPress, then reload this page.' => 'TeamVault non può scrivere nella directory di archiviazione privata, quindi i caricamenti vengono rifiutati. Rendi la directory scrivibile da WordPress, poi ricarica questa pagina.', + 'TeamVault could not create the protection files (%s) that block direct web access to the private storage directory. Uploads are refused until the directory is writable and the files exist.' => 'TeamVault non ha potuto creare i file di protezione (%s) che bloccano l\'accesso web diretto alla directory di archiviazione privata. I caricamenti vengono rifiutati finché la directory non è scrivibile e i file non esistono.', + 'This server runs Nginx, which ignores .htaccess. Confirm that a server rule denies direct requests to the private storage directory, or set a custom storage path outside the public webroot (recommended).' => 'Questo server usa Nginx, che ignora .htaccess. Verifica che una regola del server neghi le richieste dirette alla directory di archiviazione privata, oppure imposta un percorso di archiviazione personalizzato all\'esterno della webroot pubblica (consigliato).', + 'Upload refused: TeamVault could not create the protection files (%s) in the private storage directory. Make the directory writable, or use a storage path outside the public webroot, then retry.' => 'Caricamento rifiutato: TeamVault non ha potuto creare i file di protezione (%s) nella directory di archiviazione privata. Rendi la directory scrivibile, oppure usa un percorso di archiviazione all\'esterno della webroot pubblica, poi riprova.', + 'directory not writable' => 'directory non scrivibile', 'Reindex storage' => 'Reindicizza storage', 'Restore folder and file records that still exist on disk but are missing from the database.' => 'Ripristina i record di cartelle e file che esistono ancora sul disco ma mancano nel database.', 'Record upload, download, delete and move operations for files.' => 'Registra le operazioni di upload, download, eliminazione e spostamento dei file.', @@ -542,7 +547,12 @@ class MSTV_I18n 'Previous' => 'Précédent', 'Preview is not available for this file type.' => 'L\'aperçu n\'est pas disponible pour ce type de fichier.', 'TeamVault Settings' => 'Réglages TeamVault', - 'TeamVault stores private files under WordPress uploads. Confirm your web server blocks direct access to the private-documents directory, especially on Nginx.' => 'TeamVault stocke les fichiers privés dans le répertoire uploads de WordPress. Vérifiez que votre serveur web bloque l\'accès direct au répertoire private-documents, en particulier sous Nginx.', + 'A file with this storage path already exists.' => 'Un fichier existe déjà à ce chemin de stockage.', + 'TeamVault cannot write to the private storage directory, so uploads are refused. Make the directory writable by WordPress, then reload this page.' => 'TeamVault ne peut pas écrire dans le répertoire de stockage privé, les téléversements sont donc refusés. Rendez le répertoire accessible en écriture à WordPress, puis rechargez cette page.', + 'TeamVault could not create the protection files (%s) that block direct web access to the private storage directory. Uploads are refused until the directory is writable and the files exist.' => 'TeamVault n\'a pas pu créer les fichiers de protection (%s) qui bloquent l\'accès web direct au répertoire de stockage privé. Les téléversements sont refusés tant que le répertoire n\'est pas accessible en écriture et que les fichiers n\'existent pas.', + 'This server runs Nginx, which ignores .htaccess. Confirm that a server rule denies direct requests to the private storage directory, or set a custom storage path outside the public webroot (recommended).' => 'Ce serveur utilise Nginx, qui ignore .htaccess. Vérifiez qu\'une règle serveur refuse les requêtes directes vers le répertoire de stockage privé, ou définissez un chemin de stockage personnalisé en dehors de la racine web publique (recommandé).', + 'Upload refused: TeamVault could not create the protection files (%s) in the private storage directory. Make the directory writable, or use a storage path outside the public webroot, then retry.' => 'Téléversement refusé : TeamVault n\'a pas pu créer les fichiers de protection (%s) dans le répertoire de stockage privé. Rendez le répertoire accessible en écriture, ou utilisez un chemin de stockage en dehors de la racine web publique, puis réessayez.', + 'directory not writable' => 'répertoire non accessible en écriture', 'Reindex storage' => 'Réindexer le stockage', 'Restore folder and file records that still exist on disk but are missing from the database.' => 'Restaurez les enregistrements de dossiers et de fichiers qui existent encore sur le disque mais sont absents de la base de données.', 'Record upload, download, delete and move operations for files.' => 'Enregistrez les opérations de téléversement, téléchargement, suppression et déplacement des fichiers.', @@ -858,7 +868,12 @@ class MSTV_I18n 'Previous' => 'Anterior', 'Preview is not available for this file type.' => 'La vista previa no está disponible para este tipo de archivo.', 'TeamVault Settings' => 'Ajustes de TeamVault', - 'TeamVault stores private files under WordPress uploads. Confirm your web server blocks direct access to the private-documents directory, especially on Nginx.' => 'TeamVault almacena archivos privados en el directorio uploads de WordPress. Confirma que tu servidor web bloquea el acceso directo al directorio private-documents, especialmente en Nginx.', + 'A file with this storage path already exists.' => 'Ya existe un archivo con esta ruta de almacenamiento.', + 'TeamVault cannot write to the private storage directory, so uploads are refused. Make the directory writable by WordPress, then reload this page.' => 'TeamVault no puede escribir en el directorio de almacenamiento privado, por lo que se rechazan las subidas. Haz que WordPress pueda escribir en el directorio y vuelve a cargar esta página.', + 'TeamVault could not create the protection files (%s) that block direct web access to the private storage directory. Uploads are refused until the directory is writable and the files exist.' => 'TeamVault no ha podido crear los archivos de protección (%s) que bloquean el acceso web directo al directorio de almacenamiento privado. Las subidas se rechazan hasta que el directorio sea escribible y los archivos existan.', + 'This server runs Nginx, which ignores .htaccess. Confirm that a server rule denies direct requests to the private storage directory, or set a custom storage path outside the public webroot (recommended).' => 'Este servidor usa Nginx, que ignora .htaccess. Confirma que una regla del servidor deniega las solicitudes directas al directorio de almacenamiento privado, o configura una ruta de almacenamiento personalizada fuera de la raíz web pública (recomendado).', + 'Upload refused: TeamVault could not create the protection files (%s) in the private storage directory. Make the directory writable, or use a storage path outside the public webroot, then retry.' => 'Subida rechazada: TeamVault no ha podido crear los archivos de protección (%s) en el directorio de almacenamiento privado. Haz que el directorio sea escribible, o usa una ruta de almacenamiento fuera de la raíz web pública, y vuelve a intentarlo.', + 'directory not writable' => 'directorio no escribible', 'Reindex storage' => 'Reindexar almacenamiento', 'Restore folder and file records that still exist on disk but are missing from the database.' => 'Restaura los registros de carpetas y archivos que aún existen en el disco pero faltan en la base de datos.', 'Record upload, download, delete and move operations for files.' => 'Registra las operaciones de subida, descarga, eliminación y movimiento de archivos.', @@ -1174,7 +1189,12 @@ class MSTV_I18n 'Previous' => 'Zurück', 'Preview is not available for this file type.' => 'Für diesen Dateityp ist keine Vorschau verfügbar.', 'TeamVault Settings' => 'TeamVault-Einstellungen', - 'TeamVault stores private files under WordPress uploads. Confirm your web server blocks direct access to the private-documents directory, especially on Nginx.' => 'TeamVault speichert private Dateien im WordPress-Uploads-Verzeichnis. Stellen Sie sicher, dass Ihr Webserver den direkten Zugriff auf das Verzeichnis private-documents blockiert, insbesondere unter Nginx.', + 'A file with this storage path already exists.' => 'Unter diesem Speicherpfad existiert bereits eine Datei.', + 'TeamVault cannot write to the private storage directory, so uploads are refused. Make the directory writable by WordPress, then reload this page.' => 'TeamVault kann nicht in das private Speicherverzeichnis schreiben, daher werden Uploads abgelehnt. Machen Sie das Verzeichnis für WordPress beschreibbar und laden Sie diese Seite neu.', + 'TeamVault could not create the protection files (%s) that block direct web access to the private storage directory. Uploads are refused until the directory is writable and the files exist.' => 'TeamVault konnte die Schutzdateien (%s) nicht erstellen, die den direkten Webzugriff auf das private Speicherverzeichnis blockieren. Uploads werden abgelehnt, bis das Verzeichnis beschreibbar ist und die Dateien existieren.', + 'This server runs Nginx, which ignores .htaccess. Confirm that a server rule denies direct requests to the private storage directory, or set a custom storage path outside the public webroot (recommended).' => 'Dieser Server verwendet Nginx, das .htaccess ignoriert. Stellen Sie sicher, dass eine Serverregel direkte Anfragen an das private Speicherverzeichnis verweigert, oder legen Sie einen benutzerdefinierten Speicherpfad außerhalb des öffentlichen Webroots fest (empfohlen).', + 'Upload refused: TeamVault could not create the protection files (%s) in the private storage directory. Make the directory writable, or use a storage path outside the public webroot, then retry.' => 'Upload abgelehnt: TeamVault konnte die Schutzdateien (%s) im privaten Speicherverzeichnis nicht erstellen. Machen Sie das Verzeichnis beschreibbar oder verwenden Sie einen Speicherpfad außerhalb des öffentlichen Webroots, und versuchen Sie es erneut.', + 'directory not writable' => 'Verzeichnis nicht beschreibbar', 'Reindex storage' => 'Speicher neu indizieren', 'Restore folder and file records that still exist on disk but are missing from the database.' => 'Stellen Sie Ordner- und Dateieinträge wieder her, die noch auf der Festplatte vorhanden sind, aber in der Datenbank fehlen.', 'Record upload, download, delete and move operations for files.' => 'Zeichnen Sie Upload-, Download-, Lösch- und Verschiebevorgänge für Dateien auf.', diff --git a/languages/mikesoft-teamvault.pot b/languages/mikesoft-teamvault.pot index 2bcc6a1..028cf72 100644 --- a/languages/mikesoft-teamvault.pot +++ b/languages/mikesoft-teamvault.pot @@ -584,8 +584,28 @@ msgstr "" msgid "Sponsor" msgstr "" -#: includes/class-mstv-admin.php:65 -msgid "TeamVault stores private files under WordPress uploads. Confirm your web server blocks direct access to the private-documents directory, especially on Nginx." +#: includes/class-mstv-admin.php:82 +msgid "TeamVault cannot write to the private storage directory, so uploads are refused. Make the directory writable by WordPress, then reload this page." +msgstr "" + +#: includes/class-mstv-admin.php:88 +msgid "TeamVault could not create the protection files (%s) that block direct web access to the private storage directory. Uploads are refused until the directory is writable and the files exist." +msgstr "" + +#: includes/class-mstv-admin.php:94 +msgid "This server runs Nginx, which ignores .htaccess. Confirm that a server rule denies direct requests to the private storage directory, or set a custom storage path outside the public webroot (recommended)." +msgstr "" + +#: includes/class-mstv-rest-controller.php:1033 +msgid "A file with this storage path already exists." +msgstr "" + +#: includes/class-mstv-storage.php:175 +msgid "Upload refused: TeamVault could not create the protection files (%s) in the private storage directory. Make the directory writable, or use a storage path outside the public webroot, then retry." +msgstr "" + +#: includes/class-mstv-storage.php:177 +msgid "directory not writable" msgstr "" #: includes/class-mstv-admin.php:111 diff --git a/tests/PDMDuplicateCreationTest.php b/tests/PDMDuplicateCreationTest.php index ed70f43..b444f28 100644 --- a/tests/PDMDuplicateCreationTest.php +++ b/tests/PDMDuplicateCreationTest.php @@ -40,7 +40,8 @@ public function test_create_folder_reports_conflict_when_the_sibling_exists(): v $controller = $this->buildController( $folderRepo, $this->getMockBuilder(MSTV_Storage::class)->disableOriginalConstructor()->getMock(), - $this->getMockBuilder(MSTV_Repository_Files::class)->disableOriginalConstructor()->getMock() + $this->getMockBuilder(MSTV_Repository_Files::class)->disableOriginalConstructor()->getMock(), + true ); $result = $controller->create_folder(new WP_REST_Request(['name' => 'Docs', 'parent_id' => null])); diff --git a/tests/PDMMigrationDedupeTest.php b/tests/PDMMigrationDedupeTest.php index 8a117f0..b29accf 100644 --- a/tests/PDMMigrationDedupeTest.php +++ b/tests/PDMMigrationDedupeTest.php @@ -17,6 +17,8 @@ final class FakeMigrationWpdb public array $files = []; /** @var string[] */ public array $queries = []; + /** @var array}> */ + public array $prepared = []; /** @var array */ public array $updates = []; /** @var array */ @@ -100,6 +102,11 @@ public function get_var($query) public function query($query) { [$sql, $args] = $this->split($query); + + if (is_array($query)) { + $this->prepared[] = ['sql' => $sql, 'args' => $args]; + } + $this->queries[] = $sql; if (str_contains($sql, 'ADD COLUMN parent_key')) { @@ -341,20 +348,19 @@ public function test_uniqueness_migration_deduplicates_rows_and_adds_indexes(): )); // Per-folder rules of the duplicate folder are merged onto the survivor. - self::assertNotEmpty(array_filter( - $wpdb->queries, - static fn(string $sql): bool => str_contains( - $sql, - 'UPDATE IGNORE `wp_mstv_folder_permissions` SET folder_id = 1 WHERE folder_id = 2' - ) + $permissionUpdates = array_values(array_filter( + $wpdb->prepared, + static fn(array $entry): bool => str_contains($entry['sql'], 'UPDATE IGNORE `wp_mstv_folder_permissions`') )); - self::assertNotEmpty(array_filter( - $wpdb->queries, - static fn(string $sql): bool => str_contains( - $sql, - 'DELETE FROM `wp_mstv_folder_permissions` WHERE folder_id = 2' - ) + self::assertNotEmpty($permissionUpdates); + self::assertSame([1, 2], $permissionUpdates[0]['args']); + + $permissionDeletes = array_values(array_filter( + $wpdb->prepared, + static fn(array $entry): bool => str_contains($entry['sql'], 'DELETE FROM `wp_mstv_folder_permissions`') )); + self::assertNotEmpty($permissionDeletes); + self::assertSame([2], $permissionDeletes[0]['args']); self::assertTrue(MSTV_Activator::schema_migration_applied('folder_parent_key')); self::assertTrue(MSTV_Activator::schema_migration_applied('folder_unique')); diff --git a/tests/PDMStorageProtectionTest.php b/tests/PDMStorageProtectionTest.php index 4db338d..a7ab666 100644 --- a/tests/PDMStorageProtectionTest.php +++ b/tests/PDMStorageProtectionTest.php @@ -32,6 +32,9 @@ protected function tearDown(): void public function test_failing_writer_reports_every_required_protection_file(): void { + // Start without the storage marker so the marker result is observable. + unlink($this->storagePath . '/.mstv-storage'); + $writer = static function (): bool { return false; };