Skip to content

Commit 1434256

Browse files
chubes4homeboy-ci[bot]
chubes4
and
homeboy-ci[bot]
authored
Add argv command specs to process runner (#779)
Co-authored-by: homeboy-ci[bot] <266378653+homeboy-ci[bot]@users.noreply.github.com>
1 parent b08dcaf commit 1434256

4 files changed

Lines changed: 238 additions & 19 deletions

File tree

‎inc/Support/CommandSpec.php‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
<?php
2+
/**
3+
* Command Spec
4+
*
5+
* Explicit argv-based process contract for command execution that should not be
6+
* interpreted by a shell.
7+
*
8+
* @package DataMachineCode\Support
9+
*/
10+
11+
namespace DataMachineCode\Support;
12+
13+
defined('ABSPATH') || exit;
14+
15+
final class CommandSpec {
16+
17+
/** @var list<string> */
18+
private array $argv;
19+
20+
/** @var string|null */
21+
private ?string $cwd;
22+
23+
/** @var array<string,string>|null */
24+
private ?array $env;
25+
26+
/**
27+
* @param list<string> $argv Command argv. The first item is the executable.
28+
* @param array{cwd?: string|null, env?: array<string,mixed>|null} $options Execution policy.
29+
*/
30+
private function __construct( array $argv, array $options = array() ) {
31+
$this->argv = $argv;
32+
$this->cwd = isset($options['cwd']) && is_string($options['cwd']) && '' !== $options['cwd'] ? $options['cwd'] : null;
33+
$this->env = isset($options['env']) && is_array($options['env']) ? self::normalize_env($options['env']) : null;
34+
}
35+
36+
/**
37+
* Build a command spec from argv tokens.
38+
*
39+
* @param array<int,mixed> $argv Command argv. The first item is the executable.
40+
* @param array{cwd?: string|null, env?: array<string,mixed>|null} $options Execution policy.
41+
* @return self|\WP_Error
42+
*/
43+
public static function from_argv( array $argv, array $options = array() ): self|\WP_Error {
44+
$normalized = array();
45+
foreach ( $argv as $arg ) {
46+
if ( ! is_scalar($arg) ) {
47+
return new \WP_Error('invalid_command_spec', 'Command argv entries must be scalar strings.', array( 'status' => 400 ));
48+
}
49+
50+
$arg = (string) $arg;
51+
if ( str_contains($arg, "\0") ) {
52+
return new \WP_Error('invalid_command_spec', 'Command argv entries must not contain null bytes.', array( 'status' => 400 ));
53+
}
54+
55+
$normalized[] = $arg;
56+
}
57+
58+
if ( array() === $normalized || '' === $normalized[0] ) {
59+
return new \WP_Error('invalid_command_spec', 'Command argv must include an executable.', array( 'status' => 400 ));
60+
}
61+
62+
return new self($normalized, $options);
63+
}
64+
65+
/**
66+
* @return list<string>
67+
*/
68+
public function argv(): array {
69+
return $this->argv;
70+
}
71+
72+
public function cwd(): ?string {
73+
return $this->cwd;
74+
}
75+
76+
/**
77+
* @return array<string,string>|null
78+
*/
79+
public function env(): ?array {
80+
return $this->env;
81+
}
82+
83+
/**
84+
* @return array<string,string>|null
85+
*/
86+
private static function normalize_env( array $env ): ?array {
87+
$normalized = array();
88+
foreach ( $env as $key => $value ) {
89+
if ( is_scalar($value) ) {
90+
$normalized[ (string) $key ] = (string) $value;
91+
}
92+
}
93+
94+
return array() === $normalized ? null : $normalized;
95+
}
96+
}

‎inc/Support/ProcessRunner.php‎

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
if ( ! class_exists(RuntimeCapabilities::class) ) {
1717
require_once __DIR__ . '/RuntimeCapabilities.php';
1818
}
19+
if ( ! class_exists(CommandSpec::class) ) {
20+
require_once __DIR__ . '/CommandSpec.php';
21+
}
1922

2023
final class ProcessRunner {
2124

@@ -24,18 +27,23 @@ final class ProcessRunner {
2427
/**
2528
* Execute a shell command and return a normalized result envelope.
2629
*
27-
* @param string $command Shell command to execute.
30+
* @param string|CommandSpec $command Shell command or argv command spec to execute.
2831
* @param array<string,mixed> $options Execution options.
2932
* @return array<string,mixed>|\WP_Error
3033
*/
31-
public static function run( string $command, array $options = array() ): array|\WP_Error {
34+
public static function run( string|CommandSpec $command, array $options = array() ): array|\WP_Error {
3235
$timeout_seconds = max(0, (int) ( $options['timeout_seconds'] ?? 0 ));
3336
$output_cap = max(0, (int) ( $options['output_cap_bytes'] ?? 0 ));
3437
$on_output = is_callable($options['on_output'] ?? null) ? $options['on_output'] : null;
35-
$env = isset($options['env']) && is_array($options['env']) ? $options['env'] : null;
36-
$cwd = isset($options['cwd']) && is_string($options['cwd']) && '' !== $options['cwd'] ? $options['cwd'] : null;
38+
$env = self::resolve_env($command, $options);
39+
$cwd = self::resolve_cwd($command, $options);
40+
$is_command_spec = $command instanceof CommandSpec;
41+
42+
if ( $is_command_spec && null === $command->env() && isset($options['env']) && ! is_array($options['env']) ) {
43+
return self::error($options, 'Process command environment must be an array.', array( 'status' => 400 ));
44+
}
3745

38-
if ( 0 === $timeout_seconds && null === $on_output && null === $env && null === $cwd ) {
46+
if ( ! $is_command_spec && 0 === $timeout_seconds && null === $on_output && null === $env && null === $cwd ) {
3947
return self::run_via_exec($command, $options, $output_cap);
4048
}
4149

@@ -48,7 +56,9 @@ public static function run( string $command, array $options = array() ): array|\
4856
);
4957
}
5058

51-
return self::run_via_proc_open($command, $options, $timeout_seconds, $output_cap, $on_output, $cwd, $env);
59+
$process_command = $is_command_spec ? $command->argv() : $command;
60+
61+
return self::run_via_proc_open($process_command, $options, $timeout_seconds, $output_cap, $on_output, $cwd, $env);
5262
}
5363

5464
/**
@@ -89,7 +99,7 @@ private static function run_via_exec( string $command, array $options, int $outp
8999
* @param array<string,mixed>|null $env
90100
* @return array<string,mixed>|\WP_Error
91101
*/
92-
private static function run_via_proc_open( string $command, array $options, int $timeout_seconds, int $output_cap, ?callable $on_output, ?string $cwd, ?array $env ): array|\WP_Error {
102+
private static function run_via_proc_open( string|array $command, array $options, int $timeout_seconds, int $output_cap, ?callable $on_output, ?string $cwd, ?array $env ): array|\WP_Error {
93103
$descriptor_spec = array(
94104
1 => array( 'pipe', 'w' ),
95105
2 => array( 'pipe', 'w' ),
@@ -195,6 +205,31 @@ private static function run_via_proc_open( string $command, array $options, int
195205
return $result;
196206
}
197207

208+
/**
209+
* @param string|CommandSpec $command Shell command or argv command spec.
210+
* @param array<string,mixed> $options Execution options.
211+
*/
212+
private static function resolve_cwd( string|CommandSpec $command, array $options ): ?string {
213+
if ( isset($options['cwd']) && is_string($options['cwd']) && '' !== $options['cwd'] ) {
214+
return $options['cwd'];
215+
}
216+
217+
return $command instanceof CommandSpec ? $command->cwd() : null;
218+
}
219+
220+
/**
221+
* @param string|CommandSpec $command Shell command or argv command spec.
222+
* @param array<string,mixed> $options Execution options.
223+
* @return array<string,mixed>|null
224+
*/
225+
private static function resolve_env( string|CommandSpec $command, array $options ): ?array {
226+
if ( isset($options['env']) && is_array($options['env']) ) {
227+
return $options['env'];
228+
}
229+
230+
return $command instanceof CommandSpec ? $command->env() : null;
231+
}
232+
198233
/**
199234
* @param resource $process
200235
* @param array<int,resource> $pipes

‎inc/Workspace/WorkspaceRepositoryLifecycle.php‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,19 @@
77

88
namespace DataMachineCode\Workspace;
99

10-
use DataMachineCode\Support\GitRunner;
10+
use DataMachineCode\Support\CommandSpec;
1111
use DataMachineCode\Support\GitHubRemote;
12+
use DataMachineCode\Support\GitRunner;
1213
use DataMachineCode\Support\ProcessRunner;
1314

1415
defined('ABSPATH') || exit;
1516

1617
if ( ! class_exists(ProcessRunner::class) ) {
1718
require_once dirname(__DIR__) . '/Support/ProcessRunner.php';
1819
}
20+
if ( ! class_exists(CommandSpec::class) ) {
21+
require_once dirname(__DIR__) . '/Support/CommandSpec.php';
22+
}
1923

2024
trait WorkspaceRepositoryLifecycle {
2125

@@ -242,8 +246,12 @@ public function clone_repo( string $url, ?string $name = null, array $options =
242246
return $env;
243247
}
244248

245-
$command = $this->build_clone_command($url, $repo_path, $partial_clone);
246-
$result = $this->run_clone_command($command, $progress_callback, $started_at, $env);
249+
$command = $this->build_clone_command($url, $repo_path, $partial_clone, $env);
250+
if ( is_wp_error($command) ) {
251+
return $command;
252+
}
253+
254+
$result = $this->run_clone_command($command, $progress_callback, $started_at);
247255

248256
if ( is_wp_error($result) ) {
249257
return $this->clone_failed_error($result, $name, $repo_path, $url);
@@ -265,18 +273,23 @@ public function clone_repo( string $url, ?string $name = null, array $options =
265273
* @param string $url Git clone URL.
266274
* @param string $repo_path Destination path.
267275
* @param bool $partial_clone Whether to request blobless partial clone.
268-
* @return string Shell command.
276+
* @param array<string,string>|null $env Extra process environment.
277+
* @return CommandSpec|\WP_Error Command spec.
269278
*/
270-
private function build_clone_command( string $url, string $repo_path, bool $partial_clone ): string {
271-
$args = array( 'clone', '--progress' );
279+
private function build_clone_command( string $url, string $repo_path, bool $partial_clone, ?array $env ): CommandSpec|\WP_Error {
280+
$args = array( 'git', 'clone', '--progress' );
272281
if ( $partial_clone ) {
273282
$args[] = '--filter=blob:none';
274283
}
275284

276-
$args[] = escapeshellarg($url);
277-
$args[] = escapeshellarg($repo_path);
285+
$args[] = $url;
286+
$args[] = $repo_path;
287+
288+
$env = $env ?? getenv();
289+
$env = is_array($env) ? $env : array();
290+
$env['GIT_TERMINAL_PROMPT'] = '0';
278291

279-
return 'GIT_TERMINAL_PROMPT=0 git ' . implode(' ', $args);
292+
return CommandSpec::from_argv($args, array( 'env' => $env ));
280293
}
281294

282295
/**
@@ -333,16 +346,15 @@ private function should_use_partial_clone( string $url ): bool {
333346
/**
334347
* Stream a clone command to an optional progress callback.
335348
*
336-
* @param string $command Shell command.
349+
* @param CommandSpec $command Command spec.
337350
* @param callable|null $progress_callback Optional progress callback.
338351
* @param float $started_at Clone start timestamp.
339352
* @return array{success: true, output: string}|\WP_Error
340353
*/
341-
private function run_clone_command( string $command, ?callable $progress_callback, float $started_at, ?array $env = null ): array|\WP_Error {
354+
private function run_clone_command( CommandSpec $command, ?callable $progress_callback, float $started_at ): array|\WP_Error {
342355
$result = ProcessRunner::run(
343356
$command,
344357
array(
345-
'env' => $env,
346358
'error_code' => 'clone_failed',
347359
'poll_interval_microseconds' => 100000,
348360
'on_output' => function ( string $chunk ) use ( $progress_callback, $started_at ): void {

‎tests/process-runner-command-spec.php‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
if ( ! defined('ABSPATH') ) {
6+
define('ABSPATH', __DIR__ . '/fixtures/');
7+
}
8+
9+
if ( ! class_exists('WP_Error') ) {
10+
class WP_Error {
11+
private string $code;
12+
private string $message;
13+
private array $data;
14+
15+
public function __construct( string $code = '', string $message = '', array $data = array() ) {
16+
$this->code = $code;
17+
$this->message = $message;
18+
$this->data = $data;
19+
}
20+
21+
public function get_error_code(): string {
22+
return $this->code;
23+
}
24+
25+
public function get_error_message(): string {
26+
return $this->message;
27+
}
28+
29+
public function get_error_data(): array {
30+
return $this->data;
31+
}
32+
}
33+
}
34+
35+
require_once dirname(__DIR__) . '/inc/Support/CommandSpec.php';
36+
require_once dirname(__DIR__) . '/inc/Support/RuntimeCapabilities.php';
37+
require_once dirname(__DIR__) . '/inc/Support/ProcessRunner.php';
38+
39+
use DataMachineCode\Support\CommandSpec;
40+
use DataMachineCode\Support\ProcessRunner;
41+
42+
function process_runner_assert_same( mixed $expected, mixed $actual, string $message ): void {
43+
if ( $expected !== $actual ) {
44+
throw new RuntimeException(sprintf('%s Expected %s, got %s.', $message, var_export($expected, true), var_export($actual, true)));
45+
}
46+
}
47+
48+
function process_runner_assert_not_error( mixed $result, string $message ): void {
49+
if ( $result instanceof WP_Error ) {
50+
throw new RuntimeException(sprintf('%s %s', $message, $result->get_error_message()));
51+
}
52+
}
53+
54+
$cwd = sys_get_temp_dir();
55+
56+
$spec = CommandSpec::from_argv(
57+
array( PHP_BINARY, '-r', 'fwrite(STDOUT, getenv("DMC_COMMAND_SPEC_TEST") . "|" . basename(getcwd()));' ),
58+
array(
59+
'cwd' => $cwd,
60+
'env' => array_merge(
61+
getenv() ?: array(),
62+
array( 'DMC_COMMAND_SPEC_TEST' => 'argv-ok' )
63+
),
64+
)
65+
);
66+
process_runner_assert_not_error($spec, 'CommandSpec should accept argv.');
67+
68+
$result = ProcessRunner::run($spec, array( 'separate_streams' => true ));
69+
process_runner_assert_not_error($result, 'ProcessRunner should execute argv specs.');
70+
process_runner_assert_same('argv-ok|' . basename($cwd), $result['stdout'], 'CommandSpec preserves argv, cwd, and env policy.');
71+
process_runner_assert_same('', $result['stderr'], 'CommandSpec captures stderr separately.');
72+
73+
$invalid = CommandSpec::from_argv(array());
74+
process_runner_assert_same(true, $invalid instanceof WP_Error, 'CommandSpec rejects empty argv.');
75+
76+
echo "process-runner-command-spec: ok\n";

0 commit comments

Comments
 (0)