From a035295783036931930baa471f0a5586be3a18b1 Mon Sep 17 00:00:00 2001 From: cschantz Date: Mon, 2 Mar 2026 21:49:24 -0500 Subject: [PATCH] CRITICAL FIXES: Trap handler flock unlock + user extraction cache bypass Fix #1: Duplicate trap handlers with missing flock unlock (CRITICAL) Problem: Line 32 set trap with flock unlock, line 373 overwrote it Result: Flock never unlocked, lock file stays locked Fix: Consolidated into single trap with flock unlock Impact: Prevents future invocations from being blocked Fix #2: User extraction cache being bypassed (10 locations) Problem: get_user_from_path_cached() existed but 10 places called extract_user_from_path() directly, bypassing cache Result: For 200 sites, user extraction done 200+ times without cache Fix: Replaced all 10 direct calls with cached version Locations: Lines 1308, 1364, 1687, 1836, 2051, 2180, 2369, 2537, 2700 Impact: Eliminates redundant stat calls for user extraction Fix #3: Removed duplicate first trap Problem: Line 32 had first trap that was immediately overwritten Fix: Removed with note that single trap at line 373 handles both Impact: Cleaner code, prevents confusion --- .../wordpress/wordpress-cron-manager.sh | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/modules/website/wordpress/wordpress-cron-manager.sh b/modules/website/wordpress/wordpress-cron-manager.sh index ee0031a..0e878c2 100755 --- a/modules/website/wordpress/wordpress-cron-manager.sh +++ b/modules/website/wordpress/wordpress-cron-manager.sh @@ -29,7 +29,7 @@ if ! flock -n 9; then print_error "Another instance of this script is already running" exit 1 fi -trap 'flock -u 9; rm -f "$LOCK_FILE"' EXIT INT TERM +# NOTE: Trap is set later at line ~373, MUST include flock unlock! # OPTIMIZATION: Parse command-line flags for script behavior # Support: --dry-run, --parallel, --log, --help @@ -370,7 +370,8 @@ get_wp_sites_cached() { } # Cleanup on exit (keep cache file for next invocation, only remove lock file) -trap 'rm -f "$LOCK_FILE"; rollback_cleanup' EXIT INT TERM +# CRITICAL: Must unlock flock (fd 9) before removing lock file! +trap 'flock -u 9 2>/dev/null; rm -f "$LOCK_FILE"; rollback_cleanup' EXIT INT TERM # OPTIMIZATION: User extraction caching (memoization) # extract_user_from_path() called 10 times, often for same path @@ -387,7 +388,7 @@ get_user_from_path_cached() { fi # Not in cache, extract and cache result - local user=$(extract_user_from_path "$site_path") + local user=$(get_user_from_path_cached "$site_path") USER_EXTRACTION_CACHE[$site_path]="$user" echo "$user" } @@ -1304,7 +1305,7 @@ preflight_check() { while IFS= read -r wp_config; do found_count=$((found_count + 1)) site_path=$(dirname "$wp_config") - user=$(extract_user_from_path "$site_path") + user=$(get_user_from_path_cached "$site_path") # Verify user is valid if ! user_is_valid "$user"; then @@ -1360,7 +1361,7 @@ show_installation_status() { while IFS= read -r wp_config; do count=$((count + 1)) site_path=$(dirname "$wp_config") - user=$(extract_user_from_path "$site_path") + user=$(get_user_from_path_cached "$site_path") # Check wp-cron status if disable_wp_cron_exists "$wp_config"; then @@ -1683,7 +1684,7 @@ case "$choice" in site_path=$(dirname "$config_file") # Extract user and domain based on control panel - user="$(extract_user_from_path "$site_path")" + user="$(get_user_from_path_cached "$site_path")" domain="" case "$SYS_CONTROL_PANEL" in cpanel) @@ -1832,7 +1833,7 @@ case "$choice" in # Extract site path and user site_path=$(dirname "$wp_config") - user=$(extract_user_from_path "$site_path") + user=$(get_user_from_path_cached "$site_path") # PRE-FLIGHT VALIDATION CHECKS echo "Running pre-flight validation checks..." @@ -2047,7 +2048,7 @@ case "$choice" in fi # Extract user from site path (per-site, not using $target_user assumption) - user=$(extract_user_from_path "$site_path") + user=$(get_user_from_path_cached "$site_path") if [ -z "$user" ]; then echo -e "${YELLOW}Warning: Could not extract username from $site_path${NC}" failed=$((failed + 1)) @@ -2176,7 +2177,7 @@ case "$choice" in failed=$((failed + 1)) continue fi - user=$(extract_user_from_path "$site_path") + user=$(get_user_from_path_cached "$site_path") echo -e "${BOLD}Processing:${NC} $site_path (user: $user)" @@ -2365,7 +2366,7 @@ case "$choice" in # Check for cron job site_path=$(dirname "$wp_config") - user=$(extract_user_from_path "$site_path") + user=$(get_user_from_path_cached "$site_path") if crontab -u "$user" -l 2>/dev/null | grep -q "$WP_CRON_FILENAME"; then echo -e "System cron: ${GREEN}CONFIGURED${NC}" @@ -2533,7 +2534,7 @@ case "$choice" in # Remove cron job - Multi-panel support site_path=$(dirname "$wp_config") - user=$(extract_user_from_path "$site_path") + user=$(get_user_from_path_cached "$site_path") if [ "$DRY_RUN" = "true" ]; then echo "[DRY-RUN] Would remove cron job from user crontab" @@ -2696,7 +2697,7 @@ case "$choice" in failed=$((failed + 1)) continue fi - user=$(extract_user_from_path "$site_path") + user=$(get_user_from_path_cached "$site_path") echo -e "${BOLD}Processing:${NC} $site_path (user: $user)"