From 886c0081c568cb5518934367cea464b7de37bed7 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Wed, 26 Aug 2026 20:23:40 -0600 Subject: [PATCH] Harden phpcs utilities against shell and option injection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The phpcs-pr/phpcs-push helpers passed changed filenames straight into the PHPCS shell command with only spaces escaped. A crafted filename could inject shell metacharacters, and — because PHPCS parses a leading-dash argument as an option — a name like `--bootstrap=...` could load arbitrary PHP. Escape each path with escapeshellarg() and prefix it with `./` so it can never be interpreted as an option. Also fixes two pre-existing style nits in phpcs-push (double space after a comma, extra trailing blank line) that surface now the file is linted. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/utilities/phpcs-pr | 12 ++++++------ .github/workflows/utilities/phpcs-push | 15 +++++++-------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/.github/workflows/utilities/phpcs-pr b/.github/workflows/utilities/phpcs-pr index 231930c4f8..0b714f0138 100755 --- a/.github/workflows/utilities/phpcs-pr +++ b/.github/workflows/utilities/phpcs-pr @@ -16,12 +16,6 @@ if (empty($argv[1])) { $fileList = shell_exec('git diff --name-only --diff-filter=ACMR origin/' . $argv[1] . ' HEAD'); $files = array_filter(explode("\n", $fileList)); -foreach ($files as &$file) { - if (strpos($file, ' ') !== false) { - $file = str_replace(' ', '\\ ', $file); - } -} - // no changes found in diff, early exit if (!count($files)) { fwrite(STDOUT, "\e[0;32mFound no changed files.\e[0m"); @@ -29,6 +23,12 @@ if (!count($files)) { exit(0); } +// Prefix each path with ./ so a filename beginning with '-' cannot be parsed +// as a PHPCS option, then escape it for safe shell usage. +$files = array_map(function ($file) { + return escapeshellarg('./' . $file); +}, $files); + // Run all changed files through the PHPCS code sniffer and generate a CSV report $csv = shell_exec('phpcs --colors -nq --report="csv" --extensions="php" ' . implode(' ', $files)); $lines = array_map(function ($row) { diff --git a/.github/workflows/utilities/phpcs-push b/.github/workflows/utilities/phpcs-push index fe51c44c2b..cda25164f9 100755 --- a/.github/workflows/utilities/phpcs-push +++ b/.github/workflows/utilities/phpcs-push @@ -16,12 +16,6 @@ if (empty($argv[1])) { $fileList = shell_exec('git show --name-only --pretty="" --diff-filter=ACMR ' . $argv[1]); $files = array_filter(explode("\n", $fileList)); -foreach ($files as &$file) { - if (strpos($file, ' ') !== false) { - $file = str_replace(' ', '\\ ', $file); - } -} - // no changes found in diff, early exit if (!count($files)) { fwrite(STDOUT, "\e[0;32mFound no changed files.\e[0m"); @@ -29,6 +23,12 @@ if (!count($files)) { exit(0); } +// Prefix each path with ./ so a filename beginning with '-' cannot be parsed +// as a PHPCS option, then escape it for safe shell usage. +$files = array_map(function ($file) { + return escapeshellarg('./' . $file); +}, $files); + // Run all changed files through the PHPCS code sniffer and generate a CSV report $csv = shell_exec('phpcs --colors -nq --report="csv" --extensions="php" ' . implode(' ', $files)); $lines = array_map(function ($row) { @@ -69,7 +69,7 @@ fwrite(STDERR, "\e[0;31mFound " ? '1 issue' : count($lines) . ' issues') . " with code quality.\e[0m"); -fwrite(STDERR, "\n"); +fwrite(STDERR, "\n"); foreach ($files as $file => $errors) { fwrite(STDERR, "\n"); @@ -84,4 +84,3 @@ foreach ($files as $file => $errors) { } } exit(1); -