GH#3916: Recover high-value hardening into mergeable replacement PR

.agents/scripts/content-scanner-helper.sh

@@ -33,7 +33,7 @@
 set -euo pipefail
 
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" || exit 1
-source "${SCRIPT_DIR}/shared-constants.sh" 2>/dev/null || true
+source "${SCRIPT_DIR}/shared-constants.sh" || true
 
 # Fallback colours if shared-constants.sh not loaded
 [[ -z "${RED+x}" ]] && RED='\033[0;31m'
@@ -193,8 +193,16 @@ _cs_prefilter() {
 
 	local keyword
 	for keyword in "${_CS_PREFILTER_KEYWORDS[@]}"; do
-		if [[ "$lower_content" == *"$keyword"* ]]; then
-			return 0
+		# Keywords containing regex metacharacters (.*+?|()^$) use regex matching;
+		# plain keywords use faster literal substring matching.
+		if [[ "$keyword" =~ [.*+?|()^$] ]]; then
+			if [[ "$lower_content" =~ $keyword ]]; then
+				return 0
+			fi
+		else
+			if [[ "$lower_content" == *"$keyword"* ]]; then
+				return 0
+			fi
 		fi
 	done
 
@@ -221,17 +229,35 @@ _cs_normalize_nfkc() {
 	fi
 
 	# Try python3 first (most reliable NFKC)
+	# Sentinel character preserves trailing newlines through command substitution.
+	# Bash $() strips trailing newlines, causing false "changed" detection.
+	# The sentinel is appended inside the normalizer process (not via separate
+	# printf) so the exit code reflects whether normalization succeeded.
 	if command -v python3 &>/dev/null; then
-		printf '%s' "$content" | python3 -c "
+		local py_result
+		if py_result=$(printf '%s' "$content" | python3 -c "
 import sys, unicodedata
 text = sys.stdin.read()
-sys.stdout.write(unicodedata.normalize('NFKC', text))
-" 2>/dev/null && return 0
+sys.stdout.write(unicodedata.normalize('NFKC', text) + 'x')
+"); then
+			py_result="${py_result%x}"
+			printf '%s' "$py_result"
+			return 0
+		fi
 	fi
 
 	# Try perl as fallback (Unicode::Normalize is core since 5.8)
 	if command -v perl &>/dev/null; then
-		printf '%s' "$content" | perl -MUnicode::Normalize -CS -pe '$_ = NFKC($_)' 2>/dev/null && return 0
+		local perl_result
+		if perl_result=$(printf '%s' "$content" | perl -MUnicode::Normalize -CS -e '
+			local $/;
+			my $text = <STDIN>;
+			print NFKC($text) . "x";
+		'); then
+			perl_result="${perl_result%x}"
+			printf '%s' "$perl_result"
+			return 0
+		fi
 	fi
 
 	# No normalizer available — pass through unchanged
@@ -250,12 +276,14 @@ sys.stdout.write(unicodedata.normalize('NFKC', text))
 _cs_generate_boundary_id() {
 	# Use /dev/urandom for a short unique ID (8 hex chars)
 	if [[ -r /dev/urandom ]]; then
-		od -An -tx1 -N4 /dev/urandom 2>/dev/null | tr -d ' \n'
+		od -An -tx1 -N4 /dev/urandom | tr -d '[:space:]'
 		return 0
 	fi
 
-	# Fallback: use $RANDOM (less entropy but functional)
-	printf '%04x%04x' "$RANDOM" "$RANDOM"
+	# Fallback: combine $RANDOM with PID and epoch for less predictability.
+	# $RANDOM alone is a 15-bit PRNG seeded from PID — too predictable for
+	# boundary IDs that must resist crafted payloads.
+	printf '%04x%04x%04x' "$RANDOM" "$$" "$((SECONDS % 65536))"
 	return 0
 }
 
@@ -292,18 +320,20 @@ scan_content() {
 	byte_count=$(printf '%s' "$content" | wc -c | tr -d ' ')
 	_cs_log_info "Scanning content ($byte_count bytes)"
 
-	# Layer 1: Keyword pre-filter
-	if ! _cs_prefilter "$content"; then
+	# Layer 2 first: NFKC normalization (must run before pre-filter to prevent
+	# Unicode bypass — e.g., "𝐈𝐠𝐧𝐨𝐫𝐞" normalizes to "Ignore" which the
+	# pre-filter can then match).
+	local normalized
+	normalized=$(_cs_normalize_nfkc "$content")
+
+	# Layer 1: Keyword pre-filter on NORMALIZED content
+	if ! _cs_prefilter "$normalized"; then
 		_cs_log_success "Pre-filter: no suspicious keywords found — skipping full scan"
 		echo "CLEAN"
 		return 0
 	fi
 	_cs_log_info "Pre-filter: keyword match — proceeding to full scan"
 
-	# Layer 2: NFKC normalization
-	local normalized
-	normalized=$(_cs_normalize_nfkc "$content")
-
 	# Layer 3: Delegate to prompt-guard-helper.sh
 	if [[ ! -x "$PROMPT_GUARD" ]]; then
 		_cs_log_error "prompt-guard-helper.sh not found at: $PROMPT_GUARD"

.agents/scripts/deploy-agents-on-merge.sh

@@ -154,7 +154,12 @@
 	local changed_files
 
 	if [[ -n "$since_commit" ]]; then
-		changed_files=$(git -C "$REPO_DIR" diff --name-only "$since_commit" HEAD -- '.agents/' 2>/dev/null || echo "")
+		# Validate since_commit is a valid revision (not an injected option)
+		if [[ "$since_commit" == -* ]] || ! git -C "$REPO_DIR" rev-parse --verify "$since_commit" >/dev/null 2>&1; then
+			log_error "Invalid commit reference: $since_commit"
+			return 1
+		fi
+		changed_files=$(git -C "$REPO_DIR" diff --name-only "$since_commit" HEAD -- '.agents/' || echo "")
 	else
 		# Compare deployed VERSION with repo VERSION to detect staleness
 		local repo_version deployed_version
@@ -203,7 +208,7 @@
 	else
 		# Fallback: tar-based copy
 		# Remove existing target contents first to match rsync --delete behavior
-		find "$target_scripts_dir" -mindepth 1 -delete 2>/dev/null || true
+		find "$target_scripts_dir" -mindepth 1 -delete || true
 		(cd "$source_dir" && tar cf - .) | (cd "$target_scripts_dir" && tar xf -)
 	fi
 
@@ -261,9 +266,14 @@
 		fi
 
 		# Remove existing target contents (except custom/draft) to match rsync --delete behavior
+		if [[ -z "$TARGET_DIR" ]]; then
+			log_error "TARGET_DIR is empty — refusing to run find cleanup"
+			rm -rf "$tmp_preserve"
+			return 1
+		fi
 		find "$TARGET_DIR" -mindepth 1 -maxdepth 1 \
 			! -name 'custom' ! -name 'draft' ! -name 'loop-state' \
-			-exec rm -rf {} + 2>/dev/null || true
+			-exec rm -rf {} + || true
 
 		# Copy all agents
 		(cd "$source_dir" && tar cf - --exclude='loop-state' --exclude='custom' --exclude='draft' .) |
@@ -340,15 +350,15 @@
 			mkdir -p "$target_parent"
 
 			# Copy file (catch errors instead of letting set -e abort)
-			if ! cp "$source_file" "$target_file"; then
+			if ! cp -- "$source_file" "$target_file"; then
 				log_warn "Failed to copy: $rel_path"
 				failed=$((failed + 1))
 				continue
 			fi
 
 			# Set executable if it's a script
 			if [[ "$target_file" == *.sh ]]; then
-				if ! chmod +x "$target_file"; then
+				if ! chmod -- +x "$target_file"; then
 					log_warn "Failed to set executable: $rel_path"
 					failed=$((failed + 1))
 					continue
@@ -359,7 +369,7 @@
 		elif [[ ! -e "$source_file" ]]; then
 			# File was deleted in source — remove from target
 			if [[ -f "$target_file" ]]; then
-				if ! rm -f "$target_file"; then
+				if ! rm -f -- "$target_file"; then
 					log_warn "Failed to remove deleted file: $rel_path"
 					failed=$((failed + 1))
 					continue

.agents/scripts/settings-helper.sh

@@ -218,9 +218,15 @@
 	local jq_path
 	jq_path=$(_jq_path "$key")
 
-	# Validate the key exists in defaults
+	# Validate key format: only alphanumeric, underscores, and dots allowed
+	if [[ ! "$key" =~ ^[a-zA-Z_][a-zA-Z0-9_.]*$ ]]; then
+		print_error "Invalid key format: $key (only alphanumeric, underscores, dots)"
+		return 1
+	fi
+
+	# Validate the key exists in defaults (use getpath with key as data, not code)
 	local default_check
-	default_check=$(_generate_defaults | jq -r "$jq_path // \"__MISSING__\"" 2>/dev/null || echo "__MISSING__")
+	default_check=$(_generate_defaults | jq -r --arg k "$key" 'getpath($k | split(".")) // "__MISSING__"' 2>/dev/null || echo "__MISSING__")
 	if [[ "$default_check" == "__MISSING__" ]]; then
 		print_error "Unknown setting: $key"
 		print_info "Run 'settings-helper.sh list' to see available settings"
@@ -229,7 +235,7 @@
 
 	# Determine value type from defaults and coerce accordingly
 	local default_type
-	default_type=$(_generate_defaults | jq -r "$jq_path | type" 2>/dev/null || echo "string")
+	default_type=$(_generate_defaults | jq -r --arg k "$key" 'getpath($k | split(".")) | type' 2>/dev/null || echo "string")
 
 	local tmp_file
 	tmp_file=$(mktemp)
@@ -246,32 +252,34 @@
 			return 1
 			;;
 		esac
-		jq "$jq_path = $value" "$SETTINGS_FILE" >"$tmp_file"
+		jq --arg k "$key" --argjson v "$value" 'setpath($k | split("."); $v)' "$SETTINGS_FILE" >"$tmp_file"
 		;;
 	number)
-		if ! [[ "$value" =~ ^[0-9]+$ ]]; then
+		if ! jq -e 'tonumber' <<<"$value" >/dev/null 2>&1; then
 			print_error "Invalid number value: $value"
 			return 1
 		fi
-		jq "$jq_path = $value" "$SETTINGS_FILE" >"$tmp_file"
+		jq --arg k "$key" --argjson v "$value" 'setpath($k | split("."); $v)' "$SETTINGS_FILE" >"$tmp_file"
 		;;
 	array)
 		# Accept JSON array or comma-separated values
 		if [[ "$value" == "["* ]]; then
-			jq --argjson v "$value" "$jq_path = \$v" "$SETTINGS_FILE" >"$tmp_file"
+			jq --arg k "$key" --argjson v "$value" 'setpath($k | split("."); $v)' "$SETTINGS_FILE" >"$tmp_file"
 		else
 			# Convert comma-separated to JSON array
 			local json_array
 			json_array=$(echo "$value" | tr ',' '\n' | jq -R . | jq -s .)
-			jq --argjson v "$json_array" "$jq_path = \$v" "$SETTINGS_FILE" >"$tmp_file"
+			jq --arg k "$key" --argjson v "$json_array" 'setpath($k | split("."); $v)' "$SETTINGS_FILE" >"$tmp_file"
 		fi
 		;;
 	*)
-		jq --arg v "$value" "$jq_path = \$v" "$SETTINGS_FILE" >"$tmp_file"
+		jq --arg k "$key" --arg v "$value" 'setpath($k | split("."); $v)' "$SETTINGS_FILE" >"$tmp_file"
 		;;
 	esac
 
-	if [[ $? -eq 0 && -s "$tmp_file" ]]; then
+	# Under set -euo pipefail, jq failures exit before reaching here,
+	# so $? is always 0 — only the file-size check matters (GH#3916)
+	if [[ -s "$tmp_file" ]]; then
 		mv "$tmp_file" "$SETTINGS_FILE"
 		print_success "Set $key = $value"
 	else
@@ -310,11 +318,11 @@
 	for section in $sections; do
 		echo -e "${BLUE}[$section]${NC}"
 		local keys
-		keys=$(jq -r --arg s "$section" '.[$s] | keys[]' "$SETTINGS_FILE" 2>/dev/null)
+		keys=$(jq -r ".$section | keys[]" "$SETTINGS_FILE" 2>/dev/null)
 		for key in $keys; do
 			local full_key="${section}.${key}"
 			local value
-			value=$(jq -r --arg s "$section" --arg k "$key" '.[$s][$k]' "$SETTINGS_FILE" 2>/dev/null)
+			value=$(jq -r ".$section.$key" "$SETTINGS_FILE" 2>/dev/null)
 			local env_var
 			env_var=$(_env_var_for_key "$full_key")
 			local env_override=""