Skip to content

Commit 9c0c144

Browse files
pierluigilenocipierluigilenoci
authored
fix(scripts): harden bash scripts — escape, compat, and error handling (#1869)
* fix(scripts): harden bash scripts with escape, compat, and cleanup fixes - common.sh: complete RFC 8259 JSON escape (\b, \f, strip control chars) - common.sh: distinguish python3 success-empty vs failure in resolve_template - check-prerequisites.sh: escape doc names through json_escape in fallback path - create-new-feature.sh: remove duplicate json_escape (already in common.sh) - create-new-feature.sh: warn on stderr when spec template is not found - update-agent-context.sh: move nested function to top-level for bash 3.2 compat * fix(scripts): explicit resolve_template return code and best-effort agent updates - common.sh: resolve_template now returns 1 when no template is found, making the "not found" case explicit instead of relying on empty stdout - setup-plan.sh, create-new-feature.sh: add || true to resolve_template calls so set -e does not abort on missing templates (non-fatal) - update-agent-context.sh: accumulate errors in update_all_existing_agents instead of silently discarding them — all agents are attempted and the composite result is returned, matching the PowerShell equivalent behavior * style(scripts): add clarifying comment in resolve_template preset branch * fix(scripts): wrap python3 call in if-condition to prevent set -e abort Move the python3 command substitution in resolve_template into an if-condition so that a non-zero exit (e.g. invalid .registry JSON) does not abort the function under set -e. The fallback directory scan now executes as intended regardless of caller errexit settings. * fix(scripts): track agent file existence before update and avoid top-level globals - _update_if_new now records the path and sets _found_agent before calling update_agent_file, so that failures do not cause duplicate attempts on aliased paths (AMP/KIRO/BOB -> AGENTS_FILE) or false "no agent files found" fallback triggers - Remove top-level initialisation of _updated_paths and _found_agent; they are now created exclusively inside update_all_existing_agents, keeping the script side-effect free when sourced
1 parent 82b8ce4 commit 9c0c144

5 files changed

Lines changed: 88 additions & 73 deletions

File tree

‎scripts/bash/check-prerequisites.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ if $JSON_MODE; then
168168
if [[ ${#docs[@]} -eq 0 ]]; then
169169
json_docs="[]"
170170
else
171-
json_docs=$(printf '"%s",' "${docs[@]}")
171+
json_docs=$(for d in "${docs[@]}"; do printf '"%s",' "$(json_escape "$d")"; done)
172172
json_docs="[${json_docs%,}]"
173173
fi
174174
printf '{"FEATURE_DIR":"%s","AVAILABLE_DOCS":%s}\n' "$(json_escape "$FEATURE_DIR")" "$json_docs"

‎scripts/bash/common.sh‎

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -161,14 +161,18 @@ has_jq() {
161161
}
162162

163163
# Escape a string for safe embedding in a JSON value (fallback when jq is unavailable).
164-
# Handles backslash, double-quote, and control characters (newline, tab, carriage return).
164+
# Handles backslash, double-quote, and JSON-required control character escapes (RFC 8259).
165165
json_escape() {
166166
local s="$1"
167167
s="${s//\\/\\\\}"
168168
s="${s//\"/\\\"}"
169169
s="${s//$'\n'/\\n}"
170170
s="${s//$'\t'/\\t}"
171171
s="${s//$'\r'/\\r}"
172+
s="${s//$'\b'/\\b}"
173+
s="${s//$'\f'/\\f}"
174+
# Strip remaining control characters (U+0000–U+001F) not individually escaped above
175+
s=$(printf '%s' "$s" | tr -d '\000-\007\013\016-\037')
172176
printf '%s' "$s"
173177
}
174178

@@ -194,9 +198,11 @@ resolve_template() {
194198
if [ -d "$presets_dir" ]; then
195199
local registry_file="$presets_dir/.registry"
196200
if [ -f "$registry_file" ] && command -v python3 >/dev/null 2>&1; then
197-
# Read preset IDs sorted by priority (lower number = higher precedence)
198-
local sorted_presets
199-
sorted_presets=$(SPECKIT_REGISTRY="$registry_file" python3 -c "
201+
# Read preset IDs sorted by priority (lower number = higher precedence).
202+
# The python3 call is wrapped in an if-condition so that set -e does not
203+
# abort the function when python3 exits non-zero (e.g. invalid JSON).
204+
local sorted_presets=""
205+
if sorted_presets=$(SPECKIT_REGISTRY="$registry_file" python3 -c "
200206
import json, sys, os
201207
try:
202208
with open(os.environ['SPECKIT_REGISTRY']) as f:
@@ -206,14 +212,17 @@ try:
206212
print(pid)
207213
except Exception:
208214
sys.exit(1)
209-
" 2>/dev/null)
210-
if [ $? -eq 0 ] && [ -n "$sorted_presets" ]; then
211-
while IFS= read -r preset_id; do
212-
local candidate="$presets_dir/$preset_id/templates/${template_name}.md"
213-
[ -f "$candidate" ] && echo "$candidate" && return 0
214-
done <<< "$sorted_presets"
215+
" 2>/dev/null); then
216+
if [ -n "$sorted_presets" ]; then
217+
# python3 succeeded and returned preset IDs — search in priority order
218+
while IFS= read -r preset_id; do
219+
local candidate="$presets_dir/$preset_id/templates/${template_name}.md"
220+
[ -f "$candidate" ] && echo "$candidate" && return 0
221+
done <<< "$sorted_presets"
222+
fi
223+
# python3 succeeded but registry has no presets — nothing to search
215224
else
216-
# python3 returned empty list — fall through to directory scan
225+
# python3 failed (missing, or registry parse error) — fall back to unordered directory scan
217226
for preset in "$presets_dir"/*/; do
218227
[ -d "$preset" ] || continue
219228
local candidate="$preset/templates/${template_name}.md"
@@ -246,8 +255,9 @@ except Exception:
246255
local core="$base/${template_name}.md"
247256
[ -f "$core" ] && echo "$core" && return 0
248257

249-
# Return success with empty output so callers using set -e don't abort;
250-
# callers check [ -n "$TEMPLATE" ] to detect "not found".
251-
return 0
258+
# Template not found in any location.
259+
# Return 1 so callers can distinguish "not found" from "found".
260+
# Callers running under set -e should use: TEMPLATE=$(resolve_template ...) || true
261+
return 1
252262
}
253263

‎scripts/bash/create-new-feature.sh‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -162,17 +162,6 @@ clean_branch_name() {
162162
echo "$name" | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9]/-/g' | sed 's/-\+/-/g' | sed 's/^-//' | sed 's/-$//'
163163
}
164164

165-
# Escape a string for safe embedding in a JSON value (fallback when jq is unavailable).
166-
json_escape() {
167-
local s="$1"
168-
s="${s//\\/\\\\}"
169-
s="${s//\"/\\\"}"
170-
s="${s//$'\n'/\\n}"
171-
s="${s//$'\t'/\\t}"
172-
s="${s//$'\r'/\\r}"
173-
printf '%s' "$s"
174-
}
175-
176165
# Resolve repository root. Prefer git information when available, but fall back
177166
# to searching for repository markers so the workflow still functions in repositories that
178167
# were initialised with --no-git.
@@ -308,9 +297,14 @@ fi
308297
FEATURE_DIR="$SPECS_DIR/$BRANCH_NAME"
309298
mkdir -p "$FEATURE_DIR"
310299

311-
TEMPLATE=$(resolve_template "spec-template" "$REPO_ROOT")
300+
TEMPLATE=$(resolve_template "spec-template" "$REPO_ROOT") || true
312301
SPEC_FILE="$FEATURE_DIR/spec.md"
313-
if [ -n "$TEMPLATE" ] && [ -f "$TEMPLATE" ]; then cp "$TEMPLATE" "$SPEC_FILE"; else touch "$SPEC_FILE"; fi
302+
if [ -n "$TEMPLATE" ] && [ -f "$TEMPLATE" ]; then
303+
cp "$TEMPLATE" "$SPEC_FILE"
304+
else
305+
echo "Warning: Spec template not found; created empty spec file" >&2
306+
touch "$SPEC_FILE"
307+
fi
314308

315309
# Inform the user how to persist the feature variable in their own shell
316310
printf '# To persist: export SPECIFY_FEATURE=%q\n' "$BRANCH_NAME" >&2

‎scripts/bash/setup-plan.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ check_feature_branch "$CURRENT_BRANCH" "$HAS_GIT" || exit 1
3939
mkdir -p "$FEATURE_DIR"
4040

4141
# Copy plan template if it exists
42-
TEMPLATE=$(resolve_template "plan-template" "$REPO_ROOT")
42+
TEMPLATE=$(resolve_template "plan-template" "$REPO_ROOT") || true
4343
if [[ -n "$TEMPLATE" ]] && [[ -f "$TEMPLATE" ]]; then
4444
cp "$TEMPLATE" "$IMPL_PLAN"
4545
echo "Copied plan template to $IMPL_PLAN"

‎scripts/bash/update-agent-context.sh‎

Lines changed: 55 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -686,56 +686,67 @@ update_specific_agent() {
686686
esac
687687
}
688688

689-
update_all_existing_agents() {
690-
local found_agent=false
691-
local _updated_paths=()
692-
693-
# Helper: skip non-existent files and files already updated (dedup by
694-
# realpath so that variables pointing to the same file — e.g. AMP_FILE,
695-
# KIRO_FILE, BOB_FILE all resolving to AGENTS_FILE — are only written once).
696-
# Uses a linear array instead of associative array for bash 3.2 compatibility.
697-
update_if_new() {
698-
local file="$1" name="$2"
699-
[[ -f "$file" ]] || return 0
700-
local real_path
701-
real_path=$(realpath "$file" 2>/dev/null || echo "$file")
702-
local p
703-
if [[ ${#_updated_paths[@]} -gt 0 ]]; then
704-
for p in "${_updated_paths[@]}"; do
705-
[[ "$p" == "$real_path" ]] && return 0
706-
done
707-
fi
708-
update_agent_file "$file" "$name" || return 1
709-
_updated_paths+=("$real_path")
710-
found_agent=true
711-
}
689+
# Helper: skip non-existent files and files already updated (dedup by
690+
# realpath so that variables pointing to the same file — e.g. AMP_FILE,
691+
# KIRO_FILE, BOB_FILE all resolving to AGENTS_FILE — are only written once).
692+
# Uses a linear array instead of associative array for bash 3.2 compatibility.
693+
# Note: defined at top level because bash 3.2 does not support true
694+
# nested/local functions. _updated_paths, _found_agent, and _all_ok are
695+
# initialised exclusively inside update_all_existing_agents so that
696+
# sourcing this script has no side effects on the caller's environment.
697+
698+
_update_if_new() {
699+
local file="$1" name="$2"
700+
[[ -f "$file" ]] || return 0
701+
local real_path
702+
real_path=$(realpath "$file" 2>/dev/null || echo "$file")
703+
local p
704+
if [[ ${#_updated_paths[@]} -gt 0 ]]; then
705+
for p in "${_updated_paths[@]}"; do
706+
[[ "$p" == "$real_path" ]] && return 0
707+
done
708+
fi
709+
# Record the file as seen before attempting the update so that:
710+
# (a) aliases pointing to the same path are not retried on failure
711+
# (b) _found_agent reflects file existence, not update success
712+
_updated_paths+=("$real_path")
713+
_found_agent=true
714+
update_agent_file "$file" "$name"
715+
}
712716

713-
update_if_new "$CLAUDE_FILE" "Claude Code"
714-
update_if_new "$GEMINI_FILE" "Gemini CLI"
715-
update_if_new "$COPILOT_FILE" "GitHub Copilot"
716-
update_if_new "$CURSOR_FILE" "Cursor IDE"
717-
update_if_new "$QWEN_FILE" "Qwen Code"
718-
update_if_new "$AGENTS_FILE" "Codex/opencode"
719-
update_if_new "$AMP_FILE" "Amp"
720-
update_if_new "$KIRO_FILE" "Kiro CLI"
721-
update_if_new "$BOB_FILE" "IBM Bob"
722-
update_if_new "$WINDSURF_FILE" "Windsurf"
723-
update_if_new "$KILOCODE_FILE" "Kilo Code"
724-
update_if_new "$AUGGIE_FILE" "Auggie CLI"
725-
update_if_new "$ROO_FILE" "Roo Code"
726-
update_if_new "$CODEBUDDY_FILE" "CodeBuddy CLI"
727-
update_if_new "$SHAI_FILE" "SHAI"
728-
update_if_new "$TABNINE_FILE" "Tabnine CLI"
729-
update_if_new "$QODER_FILE" "Qoder CLI"
730-
update_if_new "$AGY_FILE" "Antigravity"
731-
update_if_new "$VIBE_FILE" "Mistral Vibe"
732-
update_if_new "$KIMI_FILE" "Kimi Code"
717+
update_all_existing_agents() {
718+
_found_agent=false
719+
_updated_paths=()
720+
local _all_ok=true
721+
722+
_update_if_new "$CLAUDE_FILE" "Claude Code" || _all_ok=false
723+
_update_if_new "$GEMINI_FILE" "Gemini CLI" || _all_ok=false
724+
_update_if_new "$COPILOT_FILE" "GitHub Copilot" || _all_ok=false
725+
_update_if_new "$CURSOR_FILE" "Cursor IDE" || _all_ok=false
726+
_update_if_new "$QWEN_FILE" "Qwen Code" || _all_ok=false
727+
_update_if_new "$AGENTS_FILE" "Codex/opencode" || _all_ok=false
728+
_update_if_new "$AMP_FILE" "Amp" || _all_ok=false
729+
_update_if_new "$KIRO_FILE" "Kiro CLI" || _all_ok=false
730+
_update_if_new "$BOB_FILE" "IBM Bob" || _all_ok=false
731+
_update_if_new "$WINDSURF_FILE" "Windsurf" || _all_ok=false
732+
_update_if_new "$KILOCODE_FILE" "Kilo Code" || _all_ok=false
733+
_update_if_new "$AUGGIE_FILE" "Auggie CLI" || _all_ok=false
734+
_update_if_new "$ROO_FILE" "Roo Code" || _all_ok=false
735+
_update_if_new "$CODEBUDDY_FILE" "CodeBuddy CLI" || _all_ok=false
736+
_update_if_new "$SHAI_FILE" "SHAI" || _all_ok=false
737+
_update_if_new "$TABNINE_FILE" "Tabnine CLI" || _all_ok=false
738+
_update_if_new "$QODER_FILE" "Qoder CLI" || _all_ok=false
739+
_update_if_new "$AGY_FILE" "Antigravity" || _all_ok=false
740+
_update_if_new "$VIBE_FILE" "Mistral Vibe" || _all_ok=false
741+
_update_if_new "$KIMI_FILE" "Kimi Code" || _all_ok=false
733742

734743
# If no agent files exist, create a default Claude file
735-
if [[ "$found_agent" == false ]]; then
744+
if [[ "$_found_agent" == false ]]; then
736745
log_info "No existing agent files found, creating default Claude file..."
737746
update_agent_file "$CLAUDE_FILE" "Claude Code" || return 1
738747
fi
748+
749+
[[ "$_all_ok" == true ]]
739750
}
740751
print_summary() {
741752
echo

0 commit comments

Comments
 (0)