mirror of
https://github.com/github/spec-kit.git
synced 2026-03-19 11:53:08 +00:00
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
This commit is contained in:
@@ -168,7 +168,7 @@ if $JSON_MODE; then
|
|||||||
if [[ ${#docs[@]} -eq 0 ]]; then
|
if [[ ${#docs[@]} -eq 0 ]]; then
|
||||||
json_docs="[]"
|
json_docs="[]"
|
||||||
else
|
else
|
||||||
json_docs=$(printf '"%s",' "${docs[@]}")
|
json_docs=$(for d in "${docs[@]}"; do printf '"%s",' "$(json_escape "$d")"; done)
|
||||||
json_docs="[${json_docs%,}]"
|
json_docs="[${json_docs%,}]"
|
||||||
fi
|
fi
|
||||||
printf '{"FEATURE_DIR":"%s","AVAILABLE_DOCS":%s}\n' "$(json_escape "$FEATURE_DIR")" "$json_docs"
|
printf '{"FEATURE_DIR":"%s","AVAILABLE_DOCS":%s}\n' "$(json_escape "$FEATURE_DIR")" "$json_docs"
|
||||||
|
|||||||
@@ -161,7 +161,7 @@ has_jq() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
# Escape a string for safe embedding in a JSON value (fallback when jq is unavailable).
|
# Escape a string for safe embedding in a JSON value (fallback when jq is unavailable).
|
||||||
# Handles backslash, double-quote, and control characters (newline, tab, carriage return).
|
# Handles backslash, double-quote, and JSON-required control character escapes (RFC 8259).
|
||||||
json_escape() {
|
json_escape() {
|
||||||
local s="$1"
|
local s="$1"
|
||||||
s="${s//\\/\\\\}"
|
s="${s//\\/\\\\}"
|
||||||
@@ -169,6 +169,10 @@ json_escape() {
|
|||||||
s="${s//$'\n'/\\n}"
|
s="${s//$'\n'/\\n}"
|
||||||
s="${s//$'\t'/\\t}"
|
s="${s//$'\t'/\\t}"
|
||||||
s="${s//$'\r'/\\r}"
|
s="${s//$'\r'/\\r}"
|
||||||
|
s="${s//$'\b'/\\b}"
|
||||||
|
s="${s//$'\f'/\\f}"
|
||||||
|
# Strip remaining control characters (U+0000–U+001F) not individually escaped above
|
||||||
|
s=$(printf '%s' "$s" | tr -d '\000-\007\013\016-\037')
|
||||||
printf '%s' "$s"
|
printf '%s' "$s"
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -194,9 +198,11 @@ resolve_template() {
|
|||||||
if [ -d "$presets_dir" ]; then
|
if [ -d "$presets_dir" ]; then
|
||||||
local registry_file="$presets_dir/.registry"
|
local registry_file="$presets_dir/.registry"
|
||||||
if [ -f "$registry_file" ] && command -v python3 >/dev/null 2>&1; then
|
if [ -f "$registry_file" ] && command -v python3 >/dev/null 2>&1; then
|
||||||
# Read preset IDs sorted by priority (lower number = higher precedence)
|
# Read preset IDs sorted by priority (lower number = higher precedence).
|
||||||
local sorted_presets
|
# The python3 call is wrapped in an if-condition so that set -e does not
|
||||||
sorted_presets=$(SPECKIT_REGISTRY="$registry_file" python3 -c "
|
# abort the function when python3 exits non-zero (e.g. invalid JSON).
|
||||||
|
local sorted_presets=""
|
||||||
|
if sorted_presets=$(SPECKIT_REGISTRY="$registry_file" python3 -c "
|
||||||
import json, sys, os
|
import json, sys, os
|
||||||
try:
|
try:
|
||||||
with open(os.environ['SPECKIT_REGISTRY']) as f:
|
with open(os.environ['SPECKIT_REGISTRY']) as f:
|
||||||
@@ -206,14 +212,17 @@ try:
|
|||||||
print(pid)
|
print(pid)
|
||||||
except Exception:
|
except Exception:
|
||||||
sys.exit(1)
|
sys.exit(1)
|
||||||
" 2>/dev/null)
|
" 2>/dev/null); then
|
||||||
if [ $? -eq 0 ] && [ -n "$sorted_presets" ]; then
|
if [ -n "$sorted_presets" ]; then
|
||||||
while IFS= read -r preset_id; do
|
# python3 succeeded and returned preset IDs — search in priority order
|
||||||
local candidate="$presets_dir/$preset_id/templates/${template_name}.md"
|
while IFS= read -r preset_id; do
|
||||||
[ -f "$candidate" ] && echo "$candidate" && return 0
|
local candidate="$presets_dir/$preset_id/templates/${template_name}.md"
|
||||||
done <<< "$sorted_presets"
|
[ -f "$candidate" ] && echo "$candidate" && return 0
|
||||||
|
done <<< "$sorted_presets"
|
||||||
|
fi
|
||||||
|
# python3 succeeded but registry has no presets — nothing to search
|
||||||
else
|
else
|
||||||
# python3 returned empty list — fall through to directory scan
|
# python3 failed (missing, or registry parse error) — fall back to unordered directory scan
|
||||||
for preset in "$presets_dir"/*/; do
|
for preset in "$presets_dir"/*/; do
|
||||||
[ -d "$preset" ] || continue
|
[ -d "$preset" ] || continue
|
||||||
local candidate="$preset/templates/${template_name}.md"
|
local candidate="$preset/templates/${template_name}.md"
|
||||||
@@ -246,8 +255,9 @@ except Exception:
|
|||||||
local core="$base/${template_name}.md"
|
local core="$base/${template_name}.md"
|
||||||
[ -f "$core" ] && echo "$core" && return 0
|
[ -f "$core" ] && echo "$core" && return 0
|
||||||
|
|
||||||
# Return success with empty output so callers using set -e don't abort;
|
# Template not found in any location.
|
||||||
# callers check [ -n "$TEMPLATE" ] to detect "not found".
|
# Return 1 so callers can distinguish "not found" from "found".
|
||||||
return 0
|
# Callers running under set -e should use: TEMPLATE=$(resolve_template ...) || true
|
||||||
|
return 1
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -162,17 +162,6 @@ clean_branch_name() {
|
|||||||
echo "$name" | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9]/-/g' | sed 's/-\+/-/g' | sed 's/^-//' | sed 's/-$//'
|
echo "$name" | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9]/-/g' | sed 's/-\+/-/g' | sed 's/^-//' | sed 's/-$//'
|
||||||
}
|
}
|
||||||
|
|
||||||
# Escape a string for safe embedding in a JSON value (fallback when jq is unavailable).
|
|
||||||
json_escape() {
|
|
||||||
local s="$1"
|
|
||||||
s="${s//\\/\\\\}"
|
|
||||||
s="${s//\"/\\\"}"
|
|
||||||
s="${s//$'\n'/\\n}"
|
|
||||||
s="${s//$'\t'/\\t}"
|
|
||||||
s="${s//$'\r'/\\r}"
|
|
||||||
printf '%s' "$s"
|
|
||||||
}
|
|
||||||
|
|
||||||
# Resolve repository root. Prefer git information when available, but fall back
|
# Resolve repository root. Prefer git information when available, but fall back
|
||||||
# to searching for repository markers so the workflow still functions in repositories that
|
# to searching for repository markers so the workflow still functions in repositories that
|
||||||
# were initialised with --no-git.
|
# were initialised with --no-git.
|
||||||
@@ -308,9 +297,14 @@ fi
|
|||||||
FEATURE_DIR="$SPECS_DIR/$BRANCH_NAME"
|
FEATURE_DIR="$SPECS_DIR/$BRANCH_NAME"
|
||||||
mkdir -p "$FEATURE_DIR"
|
mkdir -p "$FEATURE_DIR"
|
||||||
|
|
||||||
TEMPLATE=$(resolve_template "spec-template" "$REPO_ROOT")
|
TEMPLATE=$(resolve_template "spec-template" "$REPO_ROOT") || true
|
||||||
SPEC_FILE="$FEATURE_DIR/spec.md"
|
SPEC_FILE="$FEATURE_DIR/spec.md"
|
||||||
if [ -n "$TEMPLATE" ] && [ -f "$TEMPLATE" ]; then cp "$TEMPLATE" "$SPEC_FILE"; else touch "$SPEC_FILE"; fi
|
if [ -n "$TEMPLATE" ] && [ -f "$TEMPLATE" ]; then
|
||||||
|
cp "$TEMPLATE" "$SPEC_FILE"
|
||||||
|
else
|
||||||
|
echo "Warning: Spec template not found; created empty spec file" >&2
|
||||||
|
touch "$SPEC_FILE"
|
||||||
|
fi
|
||||||
|
|
||||||
# Inform the user how to persist the feature variable in their own shell
|
# Inform the user how to persist the feature variable in their own shell
|
||||||
printf '# To persist: export SPECIFY_FEATURE=%q\n' "$BRANCH_NAME" >&2
|
printf '# To persist: export SPECIFY_FEATURE=%q\n' "$BRANCH_NAME" >&2
|
||||||
|
|||||||
@@ -39,7 +39,7 @@ check_feature_branch "$CURRENT_BRANCH" "$HAS_GIT" || exit 1
|
|||||||
mkdir -p "$FEATURE_DIR"
|
mkdir -p "$FEATURE_DIR"
|
||||||
|
|
||||||
# Copy plan template if it exists
|
# Copy plan template if it exists
|
||||||
TEMPLATE=$(resolve_template "plan-template" "$REPO_ROOT")
|
TEMPLATE=$(resolve_template "plan-template" "$REPO_ROOT") || true
|
||||||
if [[ -n "$TEMPLATE" ]] && [[ -f "$TEMPLATE" ]]; then
|
if [[ -n "$TEMPLATE" ]] && [[ -f "$TEMPLATE" ]]; then
|
||||||
cp "$TEMPLATE" "$IMPL_PLAN"
|
cp "$TEMPLATE" "$IMPL_PLAN"
|
||||||
echo "Copied plan template to $IMPL_PLAN"
|
echo "Copied plan template to $IMPL_PLAN"
|
||||||
|
|||||||
@@ -686,56 +686,67 @@ update_specific_agent() {
|
|||||||
esac
|
esac
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# Helper: skip non-existent files and files already updated (dedup by
|
||||||
|
# realpath so that variables pointing to the same file — e.g. AMP_FILE,
|
||||||
|
# KIRO_FILE, BOB_FILE all resolving to AGENTS_FILE — are only written once).
|
||||||
|
# Uses a linear array instead of associative array for bash 3.2 compatibility.
|
||||||
|
# Note: defined at top level because bash 3.2 does not support true
|
||||||
|
# nested/local functions. _updated_paths, _found_agent, and _all_ok are
|
||||||
|
# initialised exclusively inside update_all_existing_agents so that
|
||||||
|
# sourcing this script has no side effects on the caller's environment.
|
||||||
|
|
||||||
|
_update_if_new() {
|
||||||
|
local file="$1" name="$2"
|
||||||
|
[[ -f "$file" ]] || return 0
|
||||||
|
local real_path
|
||||||
|
real_path=$(realpath "$file" 2>/dev/null || echo "$file")
|
||||||
|
local p
|
||||||
|
if [[ ${#_updated_paths[@]} -gt 0 ]]; then
|
||||||
|
for p in "${_updated_paths[@]}"; do
|
||||||
|
[[ "$p" == "$real_path" ]] && return 0
|
||||||
|
done
|
||||||
|
fi
|
||||||
|
# Record the file as seen before attempting the update so that:
|
||||||
|
# (a) aliases pointing to the same path are not retried on failure
|
||||||
|
# (b) _found_agent reflects file existence, not update success
|
||||||
|
_updated_paths+=("$real_path")
|
||||||
|
_found_agent=true
|
||||||
|
update_agent_file "$file" "$name"
|
||||||
|
}
|
||||||
|
|
||||||
update_all_existing_agents() {
|
update_all_existing_agents() {
|
||||||
local found_agent=false
|
_found_agent=false
|
||||||
local _updated_paths=()
|
_updated_paths=()
|
||||||
|
local _all_ok=true
|
||||||
|
|
||||||
# Helper: skip non-existent files and files already updated (dedup by
|
_update_if_new "$CLAUDE_FILE" "Claude Code" || _all_ok=false
|
||||||
# realpath so that variables pointing to the same file — e.g. AMP_FILE,
|
_update_if_new "$GEMINI_FILE" "Gemini CLI" || _all_ok=false
|
||||||
# KIRO_FILE, BOB_FILE all resolving to AGENTS_FILE — are only written once).
|
_update_if_new "$COPILOT_FILE" "GitHub Copilot" || _all_ok=false
|
||||||
# Uses a linear array instead of associative array for bash 3.2 compatibility.
|
_update_if_new "$CURSOR_FILE" "Cursor IDE" || _all_ok=false
|
||||||
update_if_new() {
|
_update_if_new "$QWEN_FILE" "Qwen Code" || _all_ok=false
|
||||||
local file="$1" name="$2"
|
_update_if_new "$AGENTS_FILE" "Codex/opencode" || _all_ok=false
|
||||||
[[ -f "$file" ]] || return 0
|
_update_if_new "$AMP_FILE" "Amp" || _all_ok=false
|
||||||
local real_path
|
_update_if_new "$KIRO_FILE" "Kiro CLI" || _all_ok=false
|
||||||
real_path=$(realpath "$file" 2>/dev/null || echo "$file")
|
_update_if_new "$BOB_FILE" "IBM Bob" || _all_ok=false
|
||||||
local p
|
_update_if_new "$WINDSURF_FILE" "Windsurf" || _all_ok=false
|
||||||
if [[ ${#_updated_paths[@]} -gt 0 ]]; then
|
_update_if_new "$KILOCODE_FILE" "Kilo Code" || _all_ok=false
|
||||||
for p in "${_updated_paths[@]}"; do
|
_update_if_new "$AUGGIE_FILE" "Auggie CLI" || _all_ok=false
|
||||||
[[ "$p" == "$real_path" ]] && return 0
|
_update_if_new "$ROO_FILE" "Roo Code" || _all_ok=false
|
||||||
done
|
_update_if_new "$CODEBUDDY_FILE" "CodeBuddy CLI" || _all_ok=false
|
||||||
fi
|
_update_if_new "$SHAI_FILE" "SHAI" || _all_ok=false
|
||||||
update_agent_file "$file" "$name" || return 1
|
_update_if_new "$TABNINE_FILE" "Tabnine CLI" || _all_ok=false
|
||||||
_updated_paths+=("$real_path")
|
_update_if_new "$QODER_FILE" "Qoder CLI" || _all_ok=false
|
||||||
found_agent=true
|
_update_if_new "$AGY_FILE" "Antigravity" || _all_ok=false
|
||||||
}
|
_update_if_new "$VIBE_FILE" "Mistral Vibe" || _all_ok=false
|
||||||
|
_update_if_new "$KIMI_FILE" "Kimi Code" || _all_ok=false
|
||||||
update_if_new "$CLAUDE_FILE" "Claude Code"
|
|
||||||
update_if_new "$GEMINI_FILE" "Gemini CLI"
|
|
||||||
update_if_new "$COPILOT_FILE" "GitHub Copilot"
|
|
||||||
update_if_new "$CURSOR_FILE" "Cursor IDE"
|
|
||||||
update_if_new "$QWEN_FILE" "Qwen Code"
|
|
||||||
update_if_new "$AGENTS_FILE" "Codex/opencode"
|
|
||||||
update_if_new "$AMP_FILE" "Amp"
|
|
||||||
update_if_new "$KIRO_FILE" "Kiro CLI"
|
|
||||||
update_if_new "$BOB_FILE" "IBM Bob"
|
|
||||||
update_if_new "$WINDSURF_FILE" "Windsurf"
|
|
||||||
update_if_new "$KILOCODE_FILE" "Kilo Code"
|
|
||||||
update_if_new "$AUGGIE_FILE" "Auggie CLI"
|
|
||||||
update_if_new "$ROO_FILE" "Roo Code"
|
|
||||||
update_if_new "$CODEBUDDY_FILE" "CodeBuddy CLI"
|
|
||||||
update_if_new "$SHAI_FILE" "SHAI"
|
|
||||||
update_if_new "$TABNINE_FILE" "Tabnine CLI"
|
|
||||||
update_if_new "$QODER_FILE" "Qoder CLI"
|
|
||||||
update_if_new "$AGY_FILE" "Antigravity"
|
|
||||||
update_if_new "$VIBE_FILE" "Mistral Vibe"
|
|
||||||
update_if_new "$KIMI_FILE" "Kimi Code"
|
|
||||||
|
|
||||||
# If no agent files exist, create a default Claude file
|
# If no agent files exist, create a default Claude file
|
||||||
if [[ "$found_agent" == false ]]; then
|
if [[ "$_found_agent" == false ]]; then
|
||||||
log_info "No existing agent files found, creating default Claude file..."
|
log_info "No existing agent files found, creating default Claude file..."
|
||||||
update_agent_file "$CLAUDE_FILE" "Claude Code" || return 1
|
update_agent_file "$CLAUDE_FILE" "Claude Code" || return 1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
[[ "$_all_ok" == true ]]
|
||||||
}
|
}
|
||||||
print_summary() {
|
print_summary() {
|
||||||
echo
|
echo
|
||||||
|
|||||||
Reference in New Issue
Block a user