Skip to content

Commit 71373d3

Browse files
LukeTowersclaude
LukeTowers
and
claude
committed
Harden phpcs utilities: escape filenames, skip empty diffs
Syncs the phpcs-pr/phpcs-push helpers to the canonical core Winter version: pass each changed path through escapeshellarg() before building the shell command (avoids injection via crafted filenames) and early-exit on an empty diff instead of invoking phpcs with no target (which scans the whole repo). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 4cd732d commit 71373d3

2 files changed

Lines changed: 88 additions & 86 deletions

File tree

‎.github/workflows/utilities/phpcs-pr‎

Lines changed: 44 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,25 @@
77
* of changed files. The PHPCS tests are only run against these changed files, to speed up the tests.
88
*/
99
if (empty($argv[1])) {
10-
echo 'You must provide a base branch to check this PR against.';
11-
echo "\n";
10+
fwrite(STDERR, 'You must provide a base branch to check this PR against.');
11+
fwrite(STDERR, "\n");
1212
exit(1);
1313
}
1414

1515
// Get a changelist of files from Git for this PR.
1616
$fileList = shell_exec('git diff --name-only --diff-filter=ACMR origin/' . $argv[1] . ' HEAD');
1717
$files = array_filter(explode("\n", $fileList));
1818

19-
foreach ($files as &$file) {
20-
if (strpos($file, ' ') !== false) {
21-
$file = str_replace(' ', '\\ ', $file);
22-
}
19+
// no changes found in diff, early exit
20+
if (!count($files)) {
21+
fwrite(STDOUT, "\e[0;32mFound no changed files.\e[0m");
22+
fwrite(STDOUT, "\n");
23+
exit(0);
2324
}
2425

26+
// Escape each path so filenames are passed to the shell safely.
27+
$files = array_map('escapeshellarg', $files);
28+
2529
// Run all changed files through the PHPCS code sniffer and generate a CSV report
2630
$csv = shell_exec('phpcs --colors -nq --report="csv" --extensions="php" ' . implode(' ', $files));
2731
$lines = array_map(function ($row) {
@@ -34,50 +38,47 @@ $lines = array_map(function ($row) {
3438
array_shift($lines);
3539

3640
if (!count($lines)) {
37-
echo "\e[0;32mFound no issues with code quality.\e[0m";
38-
echo "\n";
41+
fwrite(STDOUT, "\e[0;32mFound no issues with code quality.\e[0m");
42+
fwrite(STDOUT, "\n");
3943
exit(0);
40-
} else {
41-
// Group errors by file
42-
$files = [];
44+
}
4345

44-
foreach ($lines as $line) {
45-
$filename = str_replace(dirname(dirname(dirname(__DIR__))), '', $line[0]);
46+
// Group errors by file
47+
$files = [];
4648

47-
if (empty($files[$filename])) {
48-
$files[$filename] = [];
49-
}
49+
foreach ($lines as $line) {
50+
$filename = str_replace(dirname(dirname(dirname(__DIR__))), '', $line[0]);
5051

51-
$files[$filename][] = [
52-
'warning' => ($line[3] === 'warning'),
53-
'message' => $line[4],
54-
'line' => $line[1],
55-
];
52+
if (empty($files[$filename])) {
53+
$files[$filename] = [];
5654
}
5755

58-
// Render report
59-
echo "\e[0;31mFound "
60-
. ((count($lines) === 1)
61-
? '1 issue'
62-
: count($lines) . ' issues')
63-
. " with code quality.\e[0m";
64-
echo "\n";
56+
$files[$filename][] = [
57+
'warning' => (($line[3] ?? 'err') === 'warning'),
58+
'message' => $line[4] ?? 'unknown',
59+
'line' => $line[1] ?? '0',
60+
];
61+
}
62+
63+
// Render report
64+
fwrite(STDERR, "\e[0;31mFound "
65+
. ((count($lines) === 1)
66+
? '1 issue'
67+
: count($lines) . ' issues')
68+
. " with code quality.\e[0m");
69+
fwrite(STDERR, "\n");
6570

66-
foreach ($files as $file => $errors) {
67-
echo "\n";
68-
echo "\e[1;37m" . str_replace('"', '', $file) . "\e[0m";
69-
echo "\n\n";
71+
foreach ($files as $file => $errors) {
72+
fwrite(STDERR, "\n");
73+
fwrite(STDERR, "\e[1;37m" . str_replace('"', '', $file) . "\e[0m");
74+
fwrite(STDERR, "\n\n");
7075

71-
foreach ($errors as $error) {
72-
echo "\e[2m" . str_pad(' L' . $error['line'], 7) . " | \e[0m";
73-
if ($error['warning'] === false) {
74-
echo "\e[0;31mERR:\e[0m ";
75-
} else {
76-
echo "\e[1;33mWARN:\e[0m ";
77-
}
78-
echo $error['message'];
79-
echo "\n";
80-
}
76+
foreach ($errors as $error) {
77+
fwrite(STDERR, "\e[2m" . str_pad(' L' . $error['line'], 7) . " | \e[0m");
78+
fwrite(STDERR, $error['warning'] ? "\e[1;33mWARN:\e[0m " : "\e[0;31mERR:\e[0m ");
79+
fwrite(STDERR, $error['message']);
80+
fwrite(STDERR, "\n");
8181
}
82-
exit(1);
8382
}
83+
84+
exit(1);

‎.github/workflows/utilities/phpcs-push‎

Lines changed: 44 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,25 @@
77
* against these changed files, to speed up the tests.
88
*/
99
if (empty($argv[1])) {
10-
echo 'You must provide a commit SHA to check.';
11-
echo "\n";
10+
fwrite(STDERR, 'You must provide a commit SHA to check.');
11+
fwrite(STDERR, "\n");
1212
exit(1);
1313
}
1414

1515
// Get a changelist of files from Git for this push.
1616
$fileList = shell_exec('git show --name-only --pretty="" --diff-filter=ACMR ' . $argv[1]);
1717
$files = array_filter(explode("\n", $fileList));
1818

19-
foreach ($files as &$file) {
20-
if (strpos($file, ' ') !== false) {
21-
$file = str_replace(' ', '\\ ', $file);
22-
}
19+
// no changes found in diff, early exit
20+
if (!count($files)) {
21+
fwrite(STDOUT, "\e[0;32mFound no changed files.\e[0m");
22+
fwrite(STDOUT, "\n");
23+
exit(0);
2324
}
2425

26+
// Escape each path so filenames are passed to the shell safely.
27+
$files = array_map('escapeshellarg', $files);
28+
2529
// Run all changed files through the PHPCS code sniffer and generate a CSV report
2630
$csv = shell_exec('phpcs --colors -nq --report="csv" --extensions="php" ' . implode(' ', $files));
2731
$lines = array_map(function ($row) {
@@ -34,50 +38,47 @@ $lines = array_map(function ($row) {
3438
array_shift($lines);
3539

3640
if (!count($lines)) {
37-
echo "\e[0;32mFound no issues with code quality.\e[0m";
38-
echo "\n";
41+
fwrite(STDOUT, "\e[0;32mFound no issues with code quality.\e[0m");
42+
fwrite(STDOUT, "\n");
3943
exit(0);
40-
} else {
41-
// Group errors by file
42-
$files = [];
44+
}
4345

44-
foreach ($lines as $line) {
45-
$filename = str_replace(dirname(dirname(dirname(__DIR__))), '', $line[0]);
46+
// Group errors by file
47+
$files = [];
4648

47-
if (empty($files[$filename])) {
48-
$files[$filename] = [];
49-
}
49+
foreach ($lines as $line) {
50+
$filename = str_replace(dirname(dirname(dirname(__DIR__))), '', $line[0]);
5051

51-
$files[$filename][] = [
52-
'warning' => ($line[3] === 'warning'),
53-
'message' => $line[4],
54-
'line' => $line[1],
55-
];
52+
if (empty($files[$filename])) {
53+
$files[$filename] = [];
5654
}
5755

58-
// Render report
59-
echo "\e[0;31mFound "
60-
. ((count($lines) === 1)
61-
? '1 issue'
62-
: count($lines) . ' issues')
63-
. " with code quality.\e[0m";
64-
echo "\n";
56+
$files[$filename][] = [
57+
'warning' => (($line[3] ?? 'err') === 'warning'),
58+
'message' => $line[4] ?? 'unknown',
59+
'line' => $line[1] ?? '0',
60+
];
61+
}
62+
63+
// Render report
64+
fwrite(STDERR, "\e[0;31mFound "
65+
. ((count($lines) === 1)
66+
? '1 issue'
67+
: count($lines) . ' issues')
68+
. " with code quality.\e[0m");
69+
fwrite(STDERR, "\n");
6570

66-
foreach ($files as $file => $errors) {
67-
echo "\n";
68-
echo "\e[1;37m" . str_replace('"', '', $file) . "\e[0m";
69-
echo "\n\n";
71+
foreach ($files as $file => $errors) {
72+
fwrite(STDERR, "\n");
73+
fwrite(STDERR, "\e[1;37m" . str_replace('"', '', $file) . "\e[0m");
74+
fwrite(STDERR, "\n\n");
7075

71-
foreach ($errors as $error) {
72-
echo "\e[2m" . str_pad(' L' . $error['line'], 7) . " | \e[0m";
73-
if ($error['warning'] === false) {
74-
echo "\e[0;31mERR:\e[0m ";
75-
} else {
76-
echo "\e[1;33mWARN:\e[0m ";
77-
}
78-
echo $error['message'];
79-
echo "\n";
80-
}
76+
foreach ($errors as $error) {
77+
fwrite(STDERR, "\e[2m" . str_pad(' L' . $error['line'], 7) . " | \e[0m");
78+
fwrite(STDERR, $error['warning'] ? "\e[1;33mWARN:\e[0m " : "\e[0;31mERR:\e[0m ");
79+
fwrite(STDERR, $error['message']);
80+
fwrite(STDERR, "\n");
8181
}
82-
exit(1);
8382
}
83+
exit(1);
84+

0 commit comments

Comments
 (0)