Skip to content

GH#3603: fix critical quality-debt from PR #436 review feedback #4338

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alex-solovyev merged 2 commits into from
Mar 13, 2026
Merged

GH#3603: fix critical quality-debt from PR #436 review feedback #4338

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5be6774
fix(security): quote wp_command args in execute_wp_via_ssh to prevent…
alex-solovyev Mar 13, 2026
e5c4263
fix: pass wp-helper args safely through SSH
marcusquinn Mar 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 21 additions & 18 deletions .agents/scripts/wp-helper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,8 @@
# Applies server_ref resolution and SSH config host integration before connecting
execute_wp_via_ssh() {
local site_config="$1"
local wp_command="$2"
shift
local -a wp_args=("$@")

# Resolve server_ref and SSH config host aliases
site_config=$(resolve_server_ref "$site_config")
Expand Down Expand Up @@ -303,8 +304,7 @@
localwp)
# LocalWP - direct local access
local expanded_path="${local_path/#\~/$HOME}"
# shellcheck disable=SC2086 # wp_command is intentionally word-split into multiple args
(cd "$expanded_path" && wp $wp_command)
(cd "$expanded_path" && wp "${wp_args[@]}")
return $?
;;
hostinger | closte)
Expand All @@ -327,13 +327,16 @@
return 1
fi

# Use array for sshpass + ssh command (-n prevents stdin consumption in loops)
sshpass -f "$expanded_password_file" ssh -n "${ssh_identity_flag[@]}" -p "$ssh_port" "${ssh_user}@${ssh_host}" "cd $(printf %q "$wp_path") && wp $wp_command"
# Pass wp args as positional parameters to avoid shell interpolation issues
# shellcheck disable=SC2016 # $1/$@ expand on the remote shell, not locally
sshpass -f "$expanded_password_file" ssh -n "${ssh_identity_flag[@]}" -p "$ssh_port" "${ssh_user}@${ssh_host}" bash -lc 'cd "$1" && shift && wp "$@"' _ "$wp_path" "${wp_args[@]}"
return $?
;;
hetzner | cloudways | cloudron)
# SSH key-based authentication (preferred, -n prevents stdin consumption in loops)
ssh -n "${ssh_identity_flag[@]}" -p "$ssh_port" "${ssh_user}@${ssh_host}" "cd $(printf %q "$wp_path") && wp $wp_command"
# Pass wp args as positional parameters to avoid shell interpolation issues
# shellcheck disable=SC2016 # $1/$@ expand on the remote shell, not locally
ssh -n "${ssh_identity_flag[@]}" -p "$ssh_port" "${ssh_user}@${ssh_host}" bash -lc 'cd "$1" && shift && wp "$@"' _ "$wp_path" "${wp_args[@]}"
return $?
;;
*)
Expand All @@ -347,9 +350,9 @@
run_wp_command() {
local site_key="$1"
shift
local wp_command="$*"
local -a wp_args=("$@")

if [[ -z "$wp_command" ]]; then
if [[ ${#wp_args[@]} -eq 0 ]]; then
print_error "$ERROR_COMMAND_REQUIRED"
exit 1
fi
Expand All @@ -368,28 +371,28 @@
local site_type
site_type=$(echo "$site_config" | jq -r '.type')

print_info "Running on $site_name ($site_type): wp $wp_command"
print_info "Running on $site_name ($site_type): wp ${wp_args[*]}"

# Execute directly without eval
execute_wp_via_ssh "$site_config" "$wp_command"
execute_wp_via_ssh "$site_config" "${wp_args[@]}"
return $?
}

# Run WP-CLI command on all sites in a category
run_on_category() {
local category="$1"
shift
local wp_command="$*"
local -a wp_args=("$@")

if [[ -z "$wp_command" ]]; then
if [[ ${#wp_args[@]} -eq 0 ]]; then
print_error "$ERROR_COMMAND_REQUIRED"
exit 1
fi

load_config

print_info "Running on all sites in category: $category"
print_info "Command: wp $wp_command"
print_info "Command: wp ${wp_args[*]}"
echo ""

local site_keys
Expand All @@ -406,7 +409,7 @@
while IFS= read -r site_key; do
echo "----------------------------------------"
print_info "Site: $site_key"
if run_wp_command "$site_key" "$wp_command"; then
if run_wp_command "$site_key" "${wp_args[@]}"; then
((++success_count))
else
((++fail_count))
Expand All @@ -422,17 +425,17 @@

# Run WP-CLI command on all sites
run_on_all() {
local wp_command="$*"
local -a wp_args=("$@")

if [[ -z "$wp_command" ]]; then
if [[ ${#wp_args[@]} -eq 0 ]]; then
print_error "$ERROR_COMMAND_REQUIRED"
exit 1
fi

load_config

print_info "Running on ALL sites"
print_info "Command: wp $wp_command"
print_info "Command: wp ${wp_args[*]}"
echo ""

local site_keys
Expand All @@ -444,7 +447,7 @@
while IFS= read -r site_key; do
echo "----------------------------------------"
print_info "Site: $site_key"
if run_wp_command "$site_key" "$wp_command"; then
if run_wp_command "$site_key" "${wp_args[@]}"; then
((++success_count))
else
((++fail_count))
Expand Down Expand Up @@ -525,7 +528,7 @@
echo " [skip] .aidevops-tenant (not found)"
fi
if [[ -f "$WP_ACTIVE_TENANT_FILE" ]]; then
echo " [found] active-tenant = $(tr -d '[:space:]' <"$WP_ACTIVE_TENANT_FILE")"

Check warning on line 531 in .agents/scripts/wp-helper.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of using the literal '[:space:]' 4 times. 


See more on https://sonarcloud.io/project/issues?id=marcusquinn_aidevops&issues=AZzlGSK5hp2JR92TB7qv&open=AZzlGSK5hp2JR92TB7qv&pullRequest=4338
else
echo " [skip] active-tenant (not set)"
fi
Expand Down
Loading