From abc89912ef993ac6d8b7e3e326f6b958e920cce9 Mon Sep 17 00:00:00 2001 From: Anna Filina Date: Wed, 16 Jul 2025 21:48:00 -0400 Subject: [PATCH 1/7] Enhance command replacer - Use functions for easier reasoning about the code. - Compare commands to an allow list; easier to read and scalable. - Check for unsafe tokens. - Add default options so we don't have to repeat them in every COMMAND-OUTPUT. - Replace eval with ${EXECUTABLE_COMMAND[@]} to prevent injections. Example errors: `ls -l` ERROR: refusing to run arbitrary command: ls `phpcs && ls -l` ERROR: refusing unsafe token: && --- build/wiki-command-replacer.sh | 55 ++++++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/build/wiki-command-replacer.sh b/build/wiki-command-replacer.sh index 048ba0e..006834d 100755 --- a/build/wiki-command-replacer.sh +++ b/build/wiki-command-replacer.sh @@ -6,6 +6,45 @@ cd "$(dirname "$0")/.." MARKER_START='{{COMMAND-OUTPUT "' MARKER_END='"}}' +ALLOWED_COMMANDS=("phpcs" "phpcbf") +DEFAULT_OPTIONS=("--parallel=1" "--no-cache" "--no-colors" "--report-width=100" "--report=diff") + +tokenize_command() { + read -ra TOKENS <<< "$1" +} + +check_allowed_commands() { + local cmd="${TOKENS[0]}" + for allowed in "${ALLOWED_COMMANDS[@]}"; do + [[ "$cmd" == "$allowed" ]] && return 0 + done + + echo >&2 " ERROR: refusing to run arbitrary command: $cmd" + exit 1 +} + +validate_tokens() { + for token in "${TOKENS[@]}"; do + if [[ "$token" =~ [\;\|\&\$\<\>\`\\] ]]; then + echo >&2 " ERROR: refusing unsafe token: $token" + exit 1 + fi + done +} + +add_default_options() { + EXECUTABLE_COMMAND=("${TOKENS[0]}" "${DEFAULT_OPTIONS[@]}" "${TOKENS[@]:1}") +} + +execute_command() { + tokenize_command "$1" + check_allowed_commands + validate_tokens + add_default_options + + echo >&2 " INFO: running: ${EXECUTABLE_COMMAND[@]}" + ${EXECUTABLE_COMMAND[@]} +} if [[ -z "${CI:-}" ]]; then # The `_wiki` directory is created in a previous GitHub Action step. @@ -19,20 +58,10 @@ grep -lrF "${MARKER_START}" _wiki | while read -r file_to_process; do while IFS=$'\n' read -r line; do if [[ ${line} = ${MARKER_START}*${MARKER_END} ]]; then - COMMAND="${line##"${MARKER_START}"}" - COMMAND="${COMMAND%%"${MARKER_END}"}" - - if [[ "${COMMAND}" != "phpcs "* ]] && [[ "${COMMAND}" != "phpcbf "* ]]; then - echo >&2 " ERROR: refusing to run arbitrary command: ${COMMAND}" - exit 1 - fi - - #FIXME refuse to run commands with a semicolon / pipe / ampersand / sub-shell + USER_COMMAND="${line##"${MARKER_START}"}" + USER_COMMAND="${USER_COMMAND%%"${MARKER_END}"}" - echo >&2 " INFO: running: ${COMMAND}" - ( - eval "${COMMAND}" Date: Wed, 16 Jul 2025 21:58:14 -0400 Subject: [PATCH 2/7] Installed ShellCheck and fixed potential errors --- build/wiki-command-replacer.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/build/wiki-command-replacer.sh b/build/wiki-command-replacer.sh index 006834d..c002ef8 100755 --- a/build/wiki-command-replacer.sh +++ b/build/wiki-command-replacer.sh @@ -42,8 +42,9 @@ execute_command() { validate_tokens add_default_options - echo >&2 " INFO: running: ${EXECUTABLE_COMMAND[@]}" - ${EXECUTABLE_COMMAND[@]} + echo >&2 " INFO: running: " "${EXECUTABLE_COMMAND[@]}" + read -ra executable <<<"${EXECUTABLE_COMMAND[@]}" + "${executable[@]}" } if [[ -z "${CI:-}" ]]; then From 6d5fd6dfa2e0064bb2c874513a0b4a0e249bc7b8 Mon Sep 17 00:00:00 2001 From: Anna Filina Date: Wed, 16 Jul 2025 22:01:37 -0400 Subject: [PATCH 3/7] Disable checking for || This was like that when I found it, and I believe it was done on purpose. --- build/wiki-command-replacer.sh | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/build/wiki-command-replacer.sh b/build/wiki-command-replacer.sh index c002ef8..7e1cfa8 100755 --- a/build/wiki-command-replacer.sh +++ b/build/wiki-command-replacer.sh @@ -16,17 +16,17 @@ tokenize_command() { check_allowed_commands() { local cmd="${TOKENS[0]}" for allowed in "${ALLOWED_COMMANDS[@]}"; do - [[ "$cmd" == "$allowed" ]] && return 0 + [[ "${cmd}" == "${allowed}" ]] && return 0 done - echo >&2 " ERROR: refusing to run arbitrary command: $cmd" + echo >&2 " ERROR: refusing to run arbitrary command: ${cmd}" exit 1 } validate_tokens() { for token in "${TOKENS[@]}"; do - if [[ "$token" =~ [\;\|\&\$\<\>\`\\] ]]; then - echo >&2 " ERROR: refusing unsafe token: $token" + if [[ "${token}" =~ [\;\|\&\$\<\>\`\\] ]]; then + echo >&2 " ERROR: refusing unsafe token: ${token}" exit 1 fi done @@ -62,7 +62,8 @@ grep -lrF "${MARKER_START}" _wiki | while read -r file_to_process; do USER_COMMAND="${line##"${MARKER_START}"}" USER_COMMAND="${USER_COMMAND%%"${MARKER_END}"}" - execute_command "$USER_COMMAND" Date: Thu, 17 Jul 2025 13:21:25 -0400 Subject: [PATCH 4/7] Update build/wiki-command-replacer.sh Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com> --- build/wiki-command-replacer.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/wiki-command-replacer.sh b/build/wiki-command-replacer.sh index 7e1cfa8..7a8b2c3 100755 --- a/build/wiki-command-replacer.sh +++ b/build/wiki-command-replacer.sh @@ -7,7 +7,7 @@ cd "$(dirname "$0")/.." MARKER_START='{{COMMAND-OUTPUT "' MARKER_END='"}}' ALLOWED_COMMANDS=("phpcs" "phpcbf") -DEFAULT_OPTIONS=("--parallel=1" "--no-cache" "--no-colors" "--report-width=100" "--report=diff") +DEFAULT_OPTIONS=("--parallel=1" "--no-cache" "--no-colors" "--report-width=100") tokenize_command() { read -ra TOKENS <<< "$1" From 301cc9c032292c0492efb9b826faef5e06f5fa94 Mon Sep 17 00:00:00 2001 From: Anna Filina Date: Sat, 19 Jul 2025 09:01:43 -0400 Subject: [PATCH 5/7] Update build/wiki-command-replacer.sh Co-authored-by: Dan Wallis --- build/wiki-command-replacer.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/build/wiki-command-replacer.sh b/build/wiki-command-replacer.sh index 7a8b2c3..a00da1d 100755 --- a/build/wiki-command-replacer.sh +++ b/build/wiki-command-replacer.sh @@ -43,8 +43,7 @@ execute_command() { add_default_options echo >&2 " INFO: running: " "${EXECUTABLE_COMMAND[@]}" - read -ra executable <<<"${EXECUTABLE_COMMAND[@]}" - "${executable[@]}" + "${EXECUTABLE_COMMAND[@]}" } if [[ -z "${CI:-}" ]]; then From a7ec00d89bad173d4522d0d7f9714618d73b5d5d Mon Sep 17 00:00:00 2001 From: Anna Filina Date: Wed, 30 Jul 2025 17:22:04 -0400 Subject: [PATCH 6/7] Remove default options from this PR's scope --- build/wiki-command-replacer.sh | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/build/wiki-command-replacer.sh b/build/wiki-command-replacer.sh index a00da1d..0bbf905 100755 --- a/build/wiki-command-replacer.sh +++ b/build/wiki-command-replacer.sh @@ -7,7 +7,6 @@ cd "$(dirname "$0")/.." MARKER_START='{{COMMAND-OUTPUT "' MARKER_END='"}}' ALLOWED_COMMANDS=("phpcs" "phpcbf") -DEFAULT_OPTIONS=("--parallel=1" "--no-cache" "--no-colors" "--report-width=100") tokenize_command() { read -ra TOKENS <<< "$1" @@ -32,16 +31,12 @@ validate_tokens() { done } -add_default_options() { - EXECUTABLE_COMMAND=("${TOKENS[0]}" "${DEFAULT_OPTIONS[@]}" "${TOKENS[@]:1}") -} - execute_command() { tokenize_command "$1" check_allowed_commands validate_tokens - add_default_options + EXECUTABLE_COMMAND=("${TOKENS[0]}" "${TOKENS[@]:1}") echo >&2 " INFO: running: " "${EXECUTABLE_COMMAND[@]}" "${EXECUTABLE_COMMAND[@]}" } From cbf279ebc934c67394cde2352f7cad6042ebfd67 Mon Sep 17 00:00:00 2001 From: Anna Filina Date: Wed, 30 Jul 2025 17:22:59 -0400 Subject: [PATCH 7/7] Move || true to where it doesn't incorrectly trigger a CI error --- build/wiki-command-replacer.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/build/wiki-command-replacer.sh b/build/wiki-command-replacer.sh index 0bbf905..47770d6 100755 --- a/build/wiki-command-replacer.sh +++ b/build/wiki-command-replacer.sh @@ -38,7 +38,7 @@ execute_command() { EXECUTABLE_COMMAND=("${TOKENS[0]}" "${TOKENS[@]:1}") echo >&2 " INFO: running: " "${EXECUTABLE_COMMAND[@]}" - "${EXECUTABLE_COMMAND[@]}" + "${EXECUTABLE_COMMAND[@]}"