Skip to content

Commit 783e435

Browse files
chubes4homeboy-ci[bot]
chubes4
and
homeboy-ci[bot]
authored
Centralize workspace writable-root policy (#763)
* refactor: centralize workspace path policy * test: load workspace policy smoke dependency * fix: satisfy workspace policy lint --------- Co-authored-by: homeboy-ci[bot] <266378653+homeboy-ci[bot]@users.noreply.github.com>
1 parent d273242 commit 783e435

6 files changed

Lines changed: 422 additions & 372 deletions

File tree

‎inc/Workspace/RemoteWorkspaceBackend.php‎

Lines changed: 16 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,20 @@
99

1010
use DataMachineCode\Abilities\GitHubAbilities;
1111
use DataMachineCode\Support\GitHubRemote;
12-
use DataMachineCode\Support\PathSecurity;
1312

1413
defined('ABSPATH') || exit;
1514

1615
class RemoteWorkspaceBackend {
1716

17+
/**
18+
* @var WorkspacePolicy
19+
*/
20+
private WorkspacePolicy $policy;
21+
22+
public function __construct( ?WorkspacePolicy $policy = null ) {
23+
$this->policy = $policy ?? new WorkspacePolicy();
24+
}
25+
1826

1927

2028
private const OPTION = 'datamachine_code_remote_workspace_state';
@@ -187,10 +195,10 @@ private function find_worktree_handle_by_repo_branch( array $state, string $repo
187195
if ( ! is_array($worktree) ) {
188196
continue;
189197
}
190-
if ( $repo_name !== (string) ( $worktree['repo_name'] ?? '' ) ) {
198+
if ( (string) ( $worktree['repo_name'] ?? '' ) !== $repo_name ) {
191199
continue;
192200
}
193-
if ( $branch !== (string) ( $worktree['branch'] ?? '' ) ) {
201+
if ( (string) ( $worktree['branch'] ?? '' ) !== $branch ) {
194202
continue;
195203
}
196204

@@ -484,7 +492,7 @@ public function write_file( string $handle, string $path, string $content ): arr
484492
return $path;
485493
}
486494

487-
$policy_check = $this->ensure_writable_roots_allow_paths((string) $context['repo_name'], array( $path ));
495+
$policy_check = $this->policy->assert_paths_writable( (string) $context['repo_name'], array( $path ) );
488496
if ( is_wp_error($policy_check) ) {
489497
return $policy_check;
490498
}
@@ -504,101 +512,6 @@ public function write_file( string $handle, string $path, string $content ): arr
504512
);
505513
}
506514

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

622-
$policy_check = $this->ensure_writable_roots_allow_paths((string) $context['repo_name'], array( $normalized_path ));
535+
$policy_check = $this->policy->assert_paths_writable( (string) $context['repo_name'], array( $normalized_path ) );
623536
if ( is_wp_error($policy_check) ) {
624537
return $policy_check;
625538
}
@@ -692,7 +605,7 @@ public function show( string $handle ): array|\WP_Error {
692605
? '#' . $context['branch']
693606
: '' ),
694607
'branch' => '' !== (string) $context['branch'] ? (string) $context['branch'] : null,
695-
'remote' => GitHubRemote::cloneUrl((string) $context['repo']),
608+
'remote' => GitHubRemote::cloneUrl( (string) $context['repo'] ),
696609
'commit' => '' !== $context['last_commit_sha'] ? $context['last_commit_sha'] : null,
697610
'dirty' => count($files),
698611
'files' => $files,
@@ -785,7 +698,7 @@ public function git_status( string $handle ): array|\WP_Error {
785698
'is_context' => ! empty($context['read_only_context']),
786699
'path' => 'github://' . $context['repo'] . '#' . $context['branch'],
787700
'branch' => $context['branch'],
788-
'remote' => GitHubRemote::cloneUrl((string) $context['repo']),
701+
'remote' => GitHubRemote::cloneUrl( (string) $context['repo'] ),
789702
'commit' => '' !== $context['last_commit_sha'] ? $context['last_commit_sha'] : null,
790703
'dirty' => count($files),
791704
'files' => $files,
@@ -1349,7 +1262,7 @@ public function git_push( string $handle, string $remote = 'origin', ?string $br
13491262
}
13501263

13511264
$push_branch = null !== $branch && '' !== $branch ? $branch : $context['branch'];
1352-
$branch_url = '' !== $push_branch ? GitHubRemote::branchUrl((string) $context['repo'], (string) $push_branch) : null;
1265+
$branch_url = '' !== $push_branch ? GitHubRemote::branchUrl( (string) $context['repo'], (string) $push_branch ) : null;
13531266

13541267
return array(
13551268
'success' => true,

‎inc/Workspace/WorkspaceGitOperations.php‎

Lines changed: 14 additions & 154 deletions
Original file line numberDiff line numberDiff line change
@@ -1272,11 +1272,7 @@ private function get_repo_allowed_paths( string $repo_name ): array {
12721272
* @return array<int,string>
12731273
*/
12741274
private function get_repo_writable_roots( string $repo_name ): array {
1275-
$policies = $this->get_workspace_git_policies();
1276-
$repo = $policies['repos'][ $repo_name ] ?? array();
1277-
$paths = $repo['writable_roots'] ?? $repo['allowed_paths'] ?? array();
1278-
1279-
return $this->normalize_policy_paths($paths);
1275+
return ( new WorkspacePolicy() )->writable_roots_for_repo($repo_name);
12801276
}
12811277

12821278
/**
@@ -1286,10 +1282,7 @@ private function get_repo_writable_roots( string $repo_name ): array {
12861282
* @return array<int,string>
12871283
*/
12881284
private function get_repo_hidden_paths( string $repo_name ): array {
1289-
$policies = $this->get_workspace_git_policies();
1290-
$repo = $policies['repos'][ $repo_name ] ?? array();
1291-
1292-
return $this->normalize_policy_paths($repo['hidden_paths'] ?? array());
1285+
return ( new WorkspacePolicy() )->hidden_paths_for_repo($repo_name);
12931286
}
12941287

12951288
/**
@@ -1299,23 +1292,7 @@ private function get_repo_hidden_paths( string $repo_name ): array {
12991292
* @return array<int,string>
13001293
*/
13011294
private function normalize_policy_paths( mixed $paths ): array {
1302-
if ( ! is_array($paths) ) {
1303-
return array();
1304-
}
1305-
1306-
$clean = array();
1307-
foreach ( $paths as $path ) {
1308-
$normalized = trim( (string) $path);
1309-
if ( '' === $normalized ) {
1310-
continue;
1311-
}
1312-
1313-
$normalized = ltrim(str_replace('\\', '/', $normalized), '/');
1314-
$normalized = rtrim($normalized, '/');
1315-
$clean[] = $normalized;
1316-
}
1317-
1318-
return array_values(array_unique($clean));
1295+
return ( new WorkspacePolicy() )->normalize_paths($paths);
13191296
}
13201297

13211298
/**
@@ -1357,10 +1334,8 @@ private function enforce_workspace_policy( string $repo_name, string $repo_path,
13571334
* @return array<string,mixed>|null|\WP_Error
13581335
*/
13591336
private function build_workspace_policy_attestation( string $repo_name, string $repo_path, ?array $paths = null ): array|null|\WP_Error {
1360-
$writable_roots = $this->get_repo_writable_roots($repo_name);
1361-
$hidden_paths = $this->get_repo_hidden_paths($repo_name);
1362-
1363-
if ( empty($writable_roots) && empty($hidden_paths) ) {
1337+
$policy = new WorkspacePolicy();
1338+
if ( empty($policy->writable_roots_for_repo($repo_name)) && empty($policy->hidden_paths_for_repo($repo_name)) ) {
13641339
return null;
13651340
}
13661341

@@ -1374,47 +1349,15 @@ private function build_workspace_policy_attestation( string $repo_name, string $
13741349
return $ignored_files;
13751350
}
13761351

1377-
$changed_files = array_values(array_unique(array_merge($changed_files, $ignored_files)));
1378-
sort($changed_files);
1379-
1380-
$policy = array(
1381-
'writable_roots' => $writable_roots,
1382-
'hidden_paths' => $hidden_paths,
1383-
);
1384-
1385-
$real_repo = realpath($repo_path);
1386-
if ( false === $real_repo ) {
1387-
return new \WP_Error('repo_path_unavailable', 'Repository path cannot be resolved for workspace policy attestation.', array( 'status' => 500 ));
1388-
}
1389-
1390-
$violations = array();
1391-
foreach ( $changed_files as $path ) {
1392-
$violations = array_merge($violations, $this->workspace_policy_path_violations($repo_path, $real_repo, $path, $writable_roots, $hidden_paths, $ignored_files));
1393-
}
1394-
1395-
// phpcs:ignore WordPress.WP.AlternativeFunctions.json_encode_json_encode -- fallback for standalone smoke tests outside WordPress.
1396-
$policy_json = function_exists('wp_json_encode') ? wp_json_encode($policy) : json_encode($policy);
1397-
1398-
return array(
1399-
'policy_hash' => hash('sha256', (string) $policy_json),
1400-
'checked_roots' => array(
1401-
'repository' => $real_repo,
1402-
'writable_roots' => $writable_roots,
1403-
'hidden_paths' => $hidden_paths,
1404-
),
1405-
'changed_files' => $changed_files,
1406-
'violations' => $violations,
1407-
'supported_checks' => array(
1408-
'writable_roots',
1409-
'hidden_paths',
1410-
'symlink',
1411-
'gitlink',
1412-
'nested_git',
1413-
'non_regular',
1414-
'outside_realpath',
1415-
'ignored',
1416-
'hardlink',
1417-
),
1352+
return $policy->attest_changed_paths(
1353+
$repo_name,
1354+
$repo_path,
1355+
$changed_files,
1356+
$ignored_files,
1357+
function ( string $path ) use ( $repo_path ): string {
1358+
$stage = $this->run_git($repo_path, 'ls-files --stage -- ' . escapeshellarg($path));
1359+
return is_wp_error($stage) ? '' : (string) ( $stage['output'] ?? '' );
1360+
}
14181361
);
14191362
}
14201363

@@ -1498,89 +1441,6 @@ private function get_workspace_policy_publish_files( string $repo_path, string $
14981441
return array_values(array_unique($files));
14991442
}
15001443

1501-
/**
1502-
* Build all policy violations for a single relative path.
1503-
*
1504-
* @param string $repo_path Repository path.
1505-
* @param string $real_repo Real repository path.
1506-
* @param string $path Relative path.
1507-
* @param array<int,string> $writable_roots Writable roots.
1508-
* @param array<int,string> $hidden_paths Hidden paths.
1509-
* @param array<int,string> $ignored_files Ignored files.
1510-
* @return array<int,array<string,string>>
1511-
*/
1512-
private function workspace_policy_path_violations( string $repo_path, string $real_repo, string $path, array $writable_roots, array $hidden_paths, array $ignored_files ): array {
1513-
$violations = array();
1514-
$path = ltrim(str_replace('\\', '/', $path), '/');
1515-
$absolute = $repo_path . '/' . $path;
1516-
1517-
$add_violation = static function ( string $reason, string $detail = '' ) use ( &$violations, $path ): void {
1518-
$violation = array(
1519-
'path' => $path,
1520-
'reason' => $reason,
1521-
);
1522-
if ( '' !== $detail ) {
1523-
$violation['detail'] = $detail;
1524-
}
1525-
$violations[] = $violation;
1526-
};
1527-
1528-
if ( ! empty($writable_roots) && ! $this->is_path_allowed($path, $writable_roots) ) {
1529-
$add_violation('outside_writable_roots', 'Changed path is outside configured writable_roots.');
1530-
}
1531-
1532-
if ( ! empty($hidden_paths) && $this->is_path_allowed($path, $hidden_paths) ) {
1533-
$add_violation('hidden_path', 'Changed path is under configured hidden_paths.');
1534-
}
1535-
1536-
if ( '.git' === $path || str_starts_with($path, '.git/') || str_contains($path, '/.git/') || str_ends_with($path, '/.git') ) {
1537-
$add_violation('nested_git', 'Changed path includes a .git directory or file.');
1538-
}
1539-
1540-
if ( in_array($path, $ignored_files, true) ) {
1541-
$add_violation('ignored', 'Changed path is ignored by git exclude rules.');
1542-
}
1543-
1544-
if ( is_link($absolute) ) {
1545-
$add_violation('symlink', 'Changed path is a symlink.');
1546-
$target = readlink($absolute);
1547-
if ( false !== $target ) {
1548-
$target_path = str_starts_with($target, '/') ? $target : dirname($absolute) . '/' . $target;
1549-
$real_target = realpath($target_path);
1550-
if ( false === $real_target || ( $real_target !== $real_repo && ! str_starts_with($real_target, $real_repo . '/') ) ) {
1551-
$add_violation('outside_realpath', 'Symlink target resolves outside the repository.');
1552-
}
1553-
foreach ( $hidden_paths as $hidden_path ) {
1554-
$hidden_real = realpath($repo_path . '/' . $hidden_path);
1555-
if ( false !== $hidden_real && false !== $real_target && ( $real_target === $hidden_real || str_starts_with($real_target, $hidden_real . '/') ) ) {
1556-
$add_violation('hidden_path_exposure', 'Symlink target resolves under hidden_paths.');
1557-
}
1558-
}
1559-
}
1560-
} elseif ( file_exists($absolute) ) {
1561-
$real_path = realpath($absolute);
1562-
if ( false === $real_path || ( $real_path !== $real_repo && ! str_starts_with($real_path, $real_repo . '/') ) ) {
1563-
$add_violation('outside_realpath', 'Changed path resolves outside the repository.');
1564-
}
1565-
1566-
if ( ! is_file($absolute) && ! is_dir($absolute) ) {
1567-
$add_violation('non_regular', 'Changed path is neither a regular file nor a directory.');
1568-
}
1569-
1570-
$stat = @lstat($absolute); // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged
1571-
if ( is_array($stat) && is_file($absolute) && (int) ( $stat['nlink'] ?? 1 ) > 1 ) {
1572-
$add_violation('hardlink', 'Changed file has more than one hardlink.');
1573-
}
1574-
}
1575-
1576-
$stage = $this->run_git($repo_path, 'ls-files --stage -- ' . escapeshellarg($path));
1577-
if ( ! is_wp_error($stage) && preg_match('/^160000\s/', (string) ( $stage['output'] ?? '' )) ) {
1578-
$add_violation('gitlink', 'Changed path is a gitlink/submodule entry.');
1579-
}
1580-
1581-
return $violations;
1582-
}
1583-
15841444
/**
15851445
* Get fixed branch restriction for a repo.
15861446
*

0 commit comments

Comments
 (0)