Skip to content

Commit 513d149

Browse files
authored
fix: resolve GitSync wp-content paths via WP_CONTENT_DIR (#448)
* fix: resolve GitSync wp-content paths via WP_CONTENT_DIR * fix: satisfy GitSync containment path analysis * fix: match GitSync cast spacing standard
1 parent 5f835ba commit 513d149

4 files changed

Lines changed: 68 additions & 23 deletions

File tree

inc/GitSync/GitSync.php

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,11 @@ public function unbind( string $slug, bool $purge = false ): array|\WP_Error {
105105
$purged = false;
106106

107107
if ( $purge && is_dir($absolute) ) {
108-
$abspath = rtrim(ABSPATH, '/');
109-
$validation = PathSecurity::validateContainment($absolute, $abspath);
108+
$root = $binding->resolveContainmentRoot();
109+
$validation = PathSecurity::validateContainment($absolute, $root);
110110
if ( ! $validation['valid'] ) {
111111
return new \WP_Error(
112-
'path_outside_abspath',
112+
'path_outside_binding_root',
113113
sprintf('Refusing to purge "%s": %s', $absolute, $validation['message'] ?? 'containment failed'),
114114
array( 'status' => 403 )
115115
);
@@ -118,7 +118,7 @@ public function unbind( string $slug, bool $purge = false ): array|\WP_Error {
118118
// Native PHP recursion so this works on managed hosts where
119119
// exec() is disabled. RecursiveIteratorIterator with
120120
// CHILD_FIRST lets unlink/rmdir happen depth-first.
121-
$purge_err = $this->removeTree($validation['real_path']);
121+
$purge_err = $this->removeTree( (string) ( $validation['real_path'] ?? $absolute ) );
122122
if ( is_wp_error($purge_err) ) {
123123
return $purge_err;
124124
}
@@ -255,9 +255,9 @@ public function updatePolicy( string $slug, array $patch ): array|\WP_Error {
255255
* be leading-slash, no traversal, not sensitive.
256256
*/
257257
private function validateLocalPath( GitSyncBinding $binding ): true|\WP_Error {
258-
$relative = ltrim(str_replace('\\', '/', $binding->local_path), '/');
258+
$relative = GitSyncBinding::normalizeRelativePath($binding->local_path);
259259
if ( '' === $relative ) {
260-
return new \WP_Error('invalid_local_path', 'local_path cannot resolve to ABSPATH itself.', array( 'status' => 400 ));
260+
return new \WP_Error('invalid_local_path', 'local_path cannot resolve to the binding root itself.', array( 'status' => 400 ));
261261
}
262262
if ( PathSecurity::hasTraversal($relative) ) {
263263
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 {
267267
}
268268

269269
// If the target already exists, run the symlink-safe check too.
270-
$abspath = rtrim(ABSPATH, '/');
271-
$absolute = $abspath . '/' . rtrim($relative, '/');
270+
$root = $binding->resolveContainmentRoot();
271+
$absolute = $binding->resolveAbsolutePath();
272272
if ( is_dir($absolute) ) {
273-
$containment = PathSecurity::validateContainment($absolute, $abspath);
273+
$containment = PathSecurity::validateContainment($absolute, $root);
274274
if ( ! $containment['valid'] ) {
275-
return new \WP_Error('path_outside_abspath', sprintf('local_path resolves outside ABSPATH: %s', $containment['message'] ?? ''), array( 'status' => 403 ));
275+
return new \WP_Error('path_outside_binding_root', sprintf('local_path resolves outside its binding root: %s', $containment['message'] ?? ''), array( 'status' => 403 ));
276276
}
277277
}
278278

inc/GitSync/GitSyncBinding.php

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* GitSync Binding
44
*
55
* Value object for a single binding between a site-owned local directory
6-
* (relative to ABSPATH) and a GitHub repository.
6+
* and a GitHub repository.
77
*
88
* Storage: serialized inside the `datamachine_gitsync_bindings` option.
99
* Validation lives here so the shape has one authoritative definition.
@@ -212,18 +212,44 @@ public function toArray(): array {
212212
}
213213

214214
/**
215-
* Resolve `local_path` to an absolute filesystem path under ABSPATH.
215+
* Resolve `local_path` to an absolute filesystem path.
216216
*
217217
* `local_path` is stored leading-slash (e.g. `/wp-content/uploads/wiki/`)
218-
* and interpreted relative to ABSPATH. Keeping the stored form portable
219-
* lets a binding move between installs with different ABSPATHs without
220-
* a migration step.
218+
* and interpreted relative to the portable WordPress install shape. Paths
219+
* under `/wp-content/` resolve through WP_CONTENT_DIR so managed hosts whose
220+
* writable content directory lives outside ABSPATH still work.
221221
*
222222
* @return string Absolute path with no trailing slash.
223223
*/
224224
public function resolveAbsolutePath(): string {
225-
$relative = ltrim(str_replace('\\', '/', $this->local_path), '/');
226-
$abspath = rtrim(ABSPATH, '/');
227-
return $abspath . '/' . rtrim($relative, '/');
225+
$relative = self::normalizeRelativePath($this->local_path);
226+
if ( self::isWpContentPath($relative) ) {
227+
$remainder = substr($relative, strlen('wp-content'));
228+
return rtrim(WP_CONTENT_DIR, '/') . rtrim($remainder, '/');
229+
}
230+
231+
return rtrim(ABSPATH, '/') . '/' . rtrim($relative, '/');
232+
}
233+
234+
/**
235+
* Resolve the filesystem root this binding must remain inside.
236+
*
237+
* @return string Absolute root path with no trailing slash.
238+
*/
239+
public function resolveContainmentRoot(): string {
240+
$relative = self::normalizeRelativePath($this->local_path);
241+
if ( self::isWpContentPath($relative) ) {
242+
return rtrim(WP_CONTENT_DIR, '/');
243+
}
244+
245+
return rtrim(ABSPATH, '/');
246+
}
247+
248+
public static function normalizeRelativePath( string $path ): string {
249+
return ltrim(str_replace('\\', '/', $path), '/');
250+
}
251+
252+
private static function isWpContentPath( string $relative ): bool {
253+
return 'wp-content' === $relative || str_starts_with($relative, 'wp-content/');
228254
}
229255
}

inc/GitSync/GitSyncFetcher.php

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,12 @@ public function pull( GitSyncBinding $binding, array $args = array() ): array|\W
7171
}
7272

7373
// Containment belt-and-suspenders: once the dir exists, re-validate
74-
// that it really lives under ABSPATH (symlink-safe).
75-
$abspath_root = rtrim(ABSPATH, '/');
76-
$containment = PathSecurity::validateContainment($absolute, $abspath_root);
74+
// that it really lives under its binding root (symlink-safe).
75+
$containment = PathSecurity::validateContainment($absolute, $binding->resolveContainmentRoot());
7776
if ( ! $containment['valid'] ) {
78-
return new \WP_Error('path_outside_abspath', $containment['message'] ?? 'containment failed', array( 'status' => 403 ));
77+
return new \WP_Error('path_outside_binding_root', $containment['message'] ?? 'containment failed', array( 'status' => 403 ));
7978
}
80-
$absolute = $containment['real_path'];
79+
$absolute = (string) ( $containment['real_path'] ?? $absolute );
8180

8281
$tree = GitHubRemote::fetchTree($slug, $binding->branch, $pat);
8382
if ( is_wp_error($tree) ) {

tests/smoke-gitsync.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@
2121
define('ABSPATH', $scratch . '/');
2222
}
2323

24+
if (! defined('WP_CONTENT_DIR') ) {
25+
define('WP_CONTENT_DIR', rtrim($scratch, '/') . '-content');
26+
}
27+
2428
$GLOBALS['__dmc_options'] = array();
2529
$GLOBALS['__dmc_http_mock'] = array();
2630
$GLOBALS['__dmc_http_capture'] = array();
@@ -239,6 +243,22 @@ public static function apiRequest( string $method, string $url, array $body, str
239243
$assert(! is_wp_error($bound), 'bind succeeded');
240244
$assert(false === is_dir(ABSPATH . 'content/wiki/'), 'bind did not create directory');
241245

246+
$content_bound = $gs->bind(
247+
array(
248+
'slug' => 'wp-content-binding',
249+
'local_path' => '/wp-content/uploads/wiki/',
250+
'remote_url' => 'https://github.com/Automattic/a8c-wiki-woocommerce',
251+
)
252+
);
253+
$assert(! is_wp_error($content_bound), 'wp-content bind succeeded');
254+
$content_binding = ( new \DataMachineCode\GitSync\GitSyncRegistry() )->get('wp-content-binding');
255+
$assert($content_binding instanceof \DataMachineCode\GitSync\GitSyncBinding, 'wp-content binding stored');
256+
$assert(
257+
rtrim(WP_CONTENT_DIR, '/') . '/uploads/wiki' === $content_binding->resolveAbsolutePath(),
258+
'wp-content binding resolves through WP_CONTENT_DIR'
259+
);
260+
$gs->unbind('wp-content-binding');
261+
242262
$dup = $gs->bind(array( 'slug' => 'wiki', 'local_path' => '/x/', 'remote_url' => 'https://github.com/a/b' ));
243263
$assert(is_wp_error($dup) && 'binding_exists' === $dup->get_error_code(), 'refuses duplicate slug');
244264

0 commit comments

Comments
 (0)