From 1c13f399fa978e2762e8d46d19d3b9e47024e2f3 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Tue, 26 May 2026 08:31:44 -0400 Subject: [PATCH 1/3] fix: resolve GitSync wp-content paths via WP_CONTENT_DIR --- inc/GitSync/GitSync.php | 18 +++++++-------- inc/GitSync/GitSyncBinding.php | 42 +++++++++++++++++++++++++++------- inc/GitSync/GitSyncFetcher.php | 7 +++--- tests/smoke-gitsync.php | 20 ++++++++++++++++ 4 files changed, 66 insertions(+), 21 deletions(-) diff --git a/inc/GitSync/GitSync.php b/inc/GitSync/GitSync.php index 4eef9e22..e73bf097 100644 --- a/inc/GitSync/GitSync.php +++ b/inc/GitSync/GitSync.php @@ -105,11 +105,11 @@ public function unbind( string $slug, bool $purge = false ): array|\WP_Error { $purged = false; if ( $purge && is_dir($absolute) ) { - $abspath = rtrim(ABSPATH, '/'); - $validation = PathSecurity::validateContainment($absolute, $abspath); + $root = $binding->resolveContainmentRoot(); + $validation = PathSecurity::validateContainment($absolute, $root); if ( ! $validation['valid'] ) { return new \WP_Error( - 'path_outside_abspath', + 'path_outside_binding_root', sprintf('Refusing to purge "%s": %s', $absolute, $validation['message'] ?? 'containment failed'), array( 'status' => 403 ) ); @@ -255,9 +255,9 @@ public function updatePolicy( string $slug, array $patch ): array|\WP_Error { * be leading-slash, no traversal, not sensitive. */ private function validateLocalPath( GitSyncBinding $binding ): true|\WP_Error { - $relative = ltrim(str_replace('\\', '/', $binding->local_path), '/'); + $relative = GitSyncBinding::normalizeRelativePath($binding->local_path); if ( '' === $relative ) { - return new \WP_Error('invalid_local_path', 'local_path cannot resolve to ABSPATH itself.', array( 'status' => 400 )); + return new \WP_Error('invalid_local_path', 'local_path cannot resolve to the binding root itself.', array( 'status' => 400 )); } if ( PathSecurity::hasTraversal($relative) ) { return new \WP_Error('path_traversal', 'local_path contains traversal segments (`.`, `..`).', array( 'status' => 400 )); @@ -267,12 +267,12 @@ private function validateLocalPath( GitSyncBinding $binding ): true|\WP_Error { } // If the target already exists, run the symlink-safe check too. - $abspath = rtrim(ABSPATH, '/'); - $absolute = $abspath . '/' . rtrim($relative, '/'); + $root = $binding->resolveContainmentRoot(); + $absolute = $binding->resolveAbsolutePath(); if ( is_dir($absolute) ) { - $containment = PathSecurity::validateContainment($absolute, $abspath); + $containment = PathSecurity::validateContainment($absolute, $root); if ( ! $containment['valid'] ) { - return new \WP_Error('path_outside_abspath', sprintf('local_path resolves outside ABSPATH: %s', $containment['message'] ?? ''), array( 'status' => 403 )); + return new \WP_Error('path_outside_binding_root', sprintf('local_path resolves outside its binding root: %s', $containment['message'] ?? ''), array( 'status' => 403 )); } } diff --git a/inc/GitSync/GitSyncBinding.php b/inc/GitSync/GitSyncBinding.php index db3a899d..ae4c9cd9 100644 --- a/inc/GitSync/GitSyncBinding.php +++ b/inc/GitSync/GitSyncBinding.php @@ -3,7 +3,7 @@ * GitSync Binding * * Value object for a single binding between a site-owned local directory - * (relative to ABSPATH) and a GitHub repository. + * and a GitHub repository. * * Storage: serialized inside the `datamachine_gitsync_bindings` option. * Validation lives here so the shape has one authoritative definition. @@ -212,18 +212,44 @@ public function toArray(): array { } /** - * Resolve `local_path` to an absolute filesystem path under ABSPATH. + * Resolve `local_path` to an absolute filesystem path. * * `local_path` is stored leading-slash (e.g. `/wp-content/uploads/wiki/`) - * and interpreted relative to ABSPATH. Keeping the stored form portable - * lets a binding move between installs with different ABSPATHs without - * a migration step. + * and interpreted relative to the portable WordPress install shape. Paths + * under `/wp-content/` resolve through WP_CONTENT_DIR so managed hosts whose + * writable content directory lives outside ABSPATH still work. * * @return string Absolute path with no trailing slash. */ public function resolveAbsolutePath(): string { - $relative = ltrim(str_replace('\\', '/', $this->local_path), '/'); - $abspath = rtrim(ABSPATH, '/'); - return $abspath . '/' . rtrim($relative, '/'); + $relative = self::normalizeRelativePath($this->local_path); + if ( self::isWpContentPath($relative) ) { + $remainder = substr($relative, strlen('wp-content')); + return rtrim(WP_CONTENT_DIR, '/') . rtrim($remainder, '/'); + } + + return rtrim(ABSPATH, '/') . '/' . rtrim($relative, '/'); + } + + /** + * Resolve the filesystem root this binding must remain inside. + * + * @return string Absolute root path with no trailing slash. + */ + public function resolveContainmentRoot(): string { + $relative = self::normalizeRelativePath($this->local_path); + if ( self::isWpContentPath($relative) ) { + return rtrim(WP_CONTENT_DIR, '/'); + } + + return rtrim(ABSPATH, '/'); + } + + public static function normalizeRelativePath( string $path ): string { + return ltrim(str_replace('\\', '/', $path), '/'); + } + + private static function isWpContentPath( string $relative ): bool { + return 'wp-content' === $relative || str_starts_with($relative, 'wp-content/'); } } diff --git a/inc/GitSync/GitSyncFetcher.php b/inc/GitSync/GitSyncFetcher.php index 76f72bdb..210abe80 100644 --- a/inc/GitSync/GitSyncFetcher.php +++ b/inc/GitSync/GitSyncFetcher.php @@ -71,11 +71,10 @@ public function pull( GitSyncBinding $binding, array $args = array() ): array|\W } // Containment belt-and-suspenders: once the dir exists, re-validate - // that it really lives under ABSPATH (symlink-safe). - $abspath_root = rtrim(ABSPATH, '/'); - $containment = PathSecurity::validateContainment($absolute, $abspath_root); + // that it really lives under its binding root (symlink-safe). + $containment = PathSecurity::validateContainment($absolute, $binding->resolveContainmentRoot()); if ( ! $containment['valid'] ) { - return new \WP_Error('path_outside_abspath', $containment['message'] ?? 'containment failed', array( 'status' => 403 )); + return new \WP_Error('path_outside_binding_root', $containment['message'] ?? 'containment failed', array( 'status' => 403 )); } $absolute = $containment['real_path']; diff --git a/tests/smoke-gitsync.php b/tests/smoke-gitsync.php index 25644cad..95559a0c 100644 --- a/tests/smoke-gitsync.php +++ b/tests/smoke-gitsync.php @@ -21,6 +21,10 @@ define('ABSPATH', $scratch . '/'); } + if (! defined('WP_CONTENT_DIR') ) { + define('WP_CONTENT_DIR', rtrim($scratch, '/') . '-content'); + } + $GLOBALS['__dmc_options'] = array(); $GLOBALS['__dmc_http_mock'] = array(); $GLOBALS['__dmc_http_capture'] = array(); @@ -239,6 +243,22 @@ public static function apiRequest( string $method, string $url, array $body, str $assert(! is_wp_error($bound), 'bind succeeded'); $assert(false === is_dir(ABSPATH . 'content/wiki/'), 'bind did not create directory'); + $content_bound = $gs->bind( + array( + 'slug' => 'wp-content-binding', + 'local_path' => '/wp-content/uploads/wiki/', + 'remote_url' => 'https://github.com/Automattic/a8c-wiki-woocommerce', + ) + ); + $assert(! is_wp_error($content_bound), 'wp-content bind succeeded'); + $content_binding = ( new \DataMachineCode\GitSync\GitSyncRegistry() )->get('wp-content-binding'); + $assert($content_binding instanceof \DataMachineCode\GitSync\GitSyncBinding, 'wp-content binding stored'); + $assert( + rtrim(WP_CONTENT_DIR, '/') . '/uploads/wiki' === $content_binding->resolveAbsolutePath(), + 'wp-content binding resolves through WP_CONTENT_DIR' + ); + $gs->unbind('wp-content-binding'); + $dup = $gs->bind(array( 'slug' => 'wiki', 'local_path' => '/x/', 'remote_url' => 'https://github.com/a/b' )); $assert(is_wp_error($dup) && 'binding_exists' === $dup->get_error_code(), 'refuses duplicate slug'); From e21f160ff445a80f6d893b4ff52875c33bc38894 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Tue, 26 May 2026 09:04:24 -0400 Subject: [PATCH 2/3] fix: satisfy GitSync containment path analysis --- inc/GitSync/GitSync.php | 2 +- inc/GitSync/GitSyncFetcher.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/inc/GitSync/GitSync.php b/inc/GitSync/GitSync.php index e73bf097..2614d700 100644 --- a/inc/GitSync/GitSync.php +++ b/inc/GitSync/GitSync.php @@ -118,7 +118,7 @@ public function unbind( string $slug, bool $purge = false ): array|\WP_Error { // Native PHP recursion so this works on managed hosts where // exec() is disabled. RecursiveIteratorIterator with // CHILD_FIRST lets unlink/rmdir happen depth-first. - $purge_err = $this->removeTree($validation['real_path']); + $purge_err = $this->removeTree((string) ( $validation['real_path'] ?? $absolute )); if ( is_wp_error($purge_err) ) { return $purge_err; } diff --git a/inc/GitSync/GitSyncFetcher.php b/inc/GitSync/GitSyncFetcher.php index 210abe80..4ee01a4c 100644 --- a/inc/GitSync/GitSyncFetcher.php +++ b/inc/GitSync/GitSyncFetcher.php @@ -76,7 +76,7 @@ public function pull( GitSyncBinding $binding, array $args = array() ): array|\W if ( ! $containment['valid'] ) { return new \WP_Error('path_outside_binding_root', $containment['message'] ?? 'containment failed', array( 'status' => 403 )); } - $absolute = $containment['real_path']; + $absolute = (string) ( $containment['real_path'] ?? $absolute ); $tree = GitHubRemote::fetchTree($slug, $binding->branch, $pat); if ( is_wp_error($tree) ) { From 4775a3f3257f4a58cc58469c3dbe774df53aae65 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Tue, 26 May 2026 09:09:56 -0400 Subject: [PATCH 3/3] fix: match GitSync cast spacing standard --- inc/GitSync/GitSync.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inc/GitSync/GitSync.php b/inc/GitSync/GitSync.php index 2614d700..74c3854e 100644 --- a/inc/GitSync/GitSync.php +++ b/inc/GitSync/GitSync.php @@ -118,7 +118,7 @@ public function unbind( string $slug, bool $purge = false ): array|\WP_Error { // Native PHP recursion so this works on managed hosts where // exec() is disabled. RecursiveIteratorIterator with // CHILD_FIRST lets unlink/rmdir happen depth-first. - $purge_err = $this->removeTree((string) ( $validation['real_path'] ?? $absolute )); + $purge_err = $this->removeTree( (string) ( $validation['real_path'] ?? $absolute ) ); if ( is_wp_error($purge_err) ) { return $purge_err; }