Skip to content

Commit cd05375

Browse files
authored
Enforce workspace writable roots in mutation tools (#661)
1 parent db33290 commit cd05375

6 files changed

Lines changed: 371 additions & 20 deletions

File tree

inc/Support/PathSecurity.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,14 @@ public static function hasTraversal( string $path ): bool {
9292
}
9393

9494
/**
95-
* Check whether a relative path is under any of a set of prefix roots.
95+
* Check whether a relative path is under any of a set of prefix roots or glob patterns.
9696
*
9797
* Used to enforce `allowed_paths` policy when staging for commit —
9898
* a path must sit inside at least one of the configured roots to be
9999
* stageable. Roots are compared as directory prefixes (so root
100100
* `articles` matches `articles/foo.md` but not `articlesX/foo.md`),
101-
* and an exact-match counts.
101+
* exact matches count, and glob roots such as docs-star-star or
102+
* plugins-star-star-README are matched with fnmatch().
102103
*
103104
* Empty `$allowed_paths` returns false — an empty allowlist means
104105
* nothing is allowed, matching the conservative Phase 2 default.
@@ -122,6 +123,9 @@ public static function isPathAllowed( string $path, array $allowed_paths ): bool
122123
if ( $normalized === $root || str_starts_with($normalized, $root . '/') ) {
123124
return true;
124125
}
126+
if ( strpbrk($root, '*?[') && fnmatch($root, $normalized) ) {
127+
return true;
128+
}
125129
}
126130

127131
return false;

inc/Workspace/RemoteWorkspaceBackend.php

Lines changed: 113 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace DataMachineCode\Workspace;
99

1010
use DataMachineCode\Abilities\GitHubAbilities;
11+
use DataMachineCode\Support\PathSecurity;
1112

1213
defined('ABSPATH') || exit;
1314

@@ -482,6 +483,11 @@ public function write_file( string $handle, string $path, string $content ): arr
482483
return $path;
483484
}
484485

486+
$policy_check = $this->ensure_writable_roots_allow_paths((string) $context['repo_name'], array( $path ));
487+
if ( is_wp_error($policy_check) ) {
488+
return $policy_check;
489+
}
490+
485491
$state = $this->state();
486492
$state['worktrees'][ $context['handle'] ]['pending_files'][ $path ] = $content;
487493
$state['worktrees'][ $context['handle'] ]['changed_files'][ $path ] = $path;
@@ -497,6 +503,101 @@ public function write_file( string $handle, string $path, string $content ): arr
497503
);
498504
}
499505

506+
/**
507+
* Enforce configured writable roots before a remote mutation is staged.
508+
*
509+
* @param string $repo_name Repository name.
510+
* @param array<int,string> $paths Repo-relative paths.
511+
* @return true|\WP_Error
512+
*/
513+
private function ensure_writable_roots_allow_paths( string $repo_name, array $paths ): true|\WP_Error {
514+
$writable_roots = $this->get_repo_writable_roots($repo_name);
515+
if ( empty($writable_roots) ) {
516+
return true;
517+
}
518+
519+
$rejected = array();
520+
foreach ( $paths as $path ) {
521+
$relative = $this->normalize_policy_path($path);
522+
if ( '' !== $relative && ! PathSecurity::isPathAllowed($relative, $writable_roots) ) {
523+
$rejected[] = $relative;
524+
}
525+
}
526+
527+
$rejected = array_values(array_unique($rejected));
528+
if ( empty($rejected) ) {
529+
return true;
530+
}
531+
532+
return new \WP_Error(
533+
'path_not_allowed',
534+
sprintf(
535+
'Path(s) outside configured writable_roots: %s. Allowed writable_roots: %s.',
536+
implode(', ', $rejected),
537+
implode(', ', $writable_roots)
538+
),
539+
array(
540+
'status' => 403,
541+
'rejected_paths' => $rejected,
542+
'writable_roots' => $writable_roots,
543+
)
544+
);
545+
}
546+
547+
/**
548+
* @return array<int,string>
549+
*/
550+
private function get_repo_writable_roots( string $repo_name ): array {
551+
$policies = $this->get_workspace_git_policies();
552+
$repo = $policies['repos'][ $repo_name ] ?? array();
553+
$paths = $repo['writable_roots'] ?? $repo['allowed_paths'] ?? array();
554+
555+
return $this->normalize_policy_paths($paths);
556+
}
557+
558+
/**
559+
* @return array{repos: array<string, array<string, mixed>>}
560+
*/
561+
private function get_workspace_git_policies(): array {
562+
$defaults = array( 'repos' => array() );
563+
$settings = function_exists('get_option') ? get_option('datamachine_workspace_git_policies', $defaults) : $defaults;
564+
if ( ! is_array($settings) ) {
565+
$settings = $defaults;
566+
}
567+
if ( ! isset($settings['repos']) || ! is_array($settings['repos']) ) {
568+
$settings['repos'] = array();
569+
}
570+
if ( function_exists('apply_filters') ) {
571+
$settings = apply_filters('datamachine_workspace_git_policies', $settings);
572+
}
573+
574+
return isset($settings['repos']) && is_array($settings['repos']) ? $settings : $defaults;
575+
}
576+
577+
/**
578+
* @param mixed $paths Raw policy value.
579+
* @return array<int,string>
580+
*/
581+
private function normalize_policy_paths( mixed $paths ): array {
582+
if ( ! is_array($paths) ) {
583+
return array();
584+
}
585+
586+
$clean = array();
587+
foreach ( $paths as $path ) {
588+
$normalized = $this->normalize_policy_path((string) $path);
589+
if ( '' !== $normalized ) {
590+
$clean[] = $normalized;
591+
}
592+
}
593+
594+
return array_values(array_unique($clean));
595+
}
596+
597+
private function normalize_policy_path( string $path ): string {
598+
return rtrim(ltrim(str_replace('\\', '/', trim($path)), '/'), '/');
599+
}
600+
500601
/**
501602
* Stage a find-and-replace edit in the remote workspace.
502603
*
@@ -512,7 +613,17 @@ public function edit_file( string $handle, string $path, string $old_string, str
512613
return $context;
513614
}
514615

515-
$current = $this->read_file($handle, $path, PHP_INT_MAX);
616+
$normalized_path = $this->normalize_path($path);
617+
if ( is_wp_error($normalized_path) ) {
618+
return $normalized_path;
619+
}
620+
621+
$policy_check = $this->ensure_writable_roots_allow_paths((string) $context['repo_name'], array( $normalized_path ));
622+
if ( is_wp_error($policy_check) ) {
623+
return $policy_check;
624+
}
625+
626+
$current = $this->read_file($handle, $normalized_path, PHP_INT_MAX);
516627
if ( is_wp_error($current) ) {
517628
return $current;
518629
}
@@ -542,7 +653,7 @@ public function edit_file( string $handle, string $path, string $old_string, str
542653
: substr_replace($content, $new_string, $offset, strlen($old_string));
543654
}
544655

545-
$write = $this->write_file($handle, $path, $new_content);
656+
$write = $this->write_file($handle, $normalized_path, $new_content);
546657
if ( is_wp_error($write) ) {
547658
return $write;
548659
}

inc/Workspace/WorkspaceGitOperations.php

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1791,20 +1791,7 @@ private function path_matches_any_glob( string $path, array $patterns ): bool {
17911791
* @return bool
17921792
*/
17931793
private function is_path_allowed( string $path, array $allowed_paths ): bool {
1794-
$normalized = ltrim(str_replace('\\', '/', $path), '/');
1795-
1796-
foreach ( $allowed_paths as $allowed ) {
1797-
$root = ltrim(str_replace('\\', '/', (string) $allowed), '/');
1798-
if ( '' === $root ) {
1799-
continue;
1800-
}
1801-
1802-
if ( $normalized === $root || str_starts_with($normalized, $root . '/') ) {
1803-
return true;
1804-
}
1805-
}
1806-
1807-
return false;
1794+
return PathSecurity::isPathAllowed($path, $allowed_paths);
18081795
}
18091796

18101797
/**

0 commit comments

Comments
 (0)