fix: harden bash scripts against shell injection and improve robustness (#1809)

- Replace eval of unquoted get_feature_paths output with safe pattern:
  capture into variable, check return code, then eval quoted result
- Use printf '%q' in get_feature_paths to safely emit shell assignments,
  preventing injection via paths containing quotes or metacharacters
- Add json_escape() helper for printf JSON fallback paths, handling
  backslash, double-quote, and control characters when jq is unavailable
- Use jq -cn for safe JSON construction with proper escaping when
  available, with printf + json_escape() fallback
- Replace declare -A (bash 4+) with indexed array for bash 3.2
  compatibility (macOS default)
- Use inline command -v jq check in create-new-feature.sh since it
  does not source common.sh
- Guard trap cleanup against re-entrant invocation by disarming traps
  at entry
- Use printf '%q' for shell-escaped branch names in user-facing output
- Return failure instead of silently returning wrong path on ambiguous
  spec directory matches
- Deduplicate agent file updates via realpath to prevent multiple writes
  to the same file (e.g. AGENTS.md aliased by multiple variables)
This commit is contained in:
Pierluigi Lenoci
2026-03-13 16:47:17 +01:00
committed by GitHub
parent 017e1c4c2f
commit 46bc65b1ce
5 changed files with 178 additions and 149 deletions

View File

@@ -120,7 +120,7 @@ find_feature_dir_by_prefix() {
# Multiple matches - this shouldn't happen with proper naming convention
echo "ERROR: Multiple spec directories found with prefix '$prefix': ${matches[*]}" >&2
echo "Please ensure only one spec directory exists per numeric prefix." >&2
echo "$specs_dir/$branch_name" # Return something to avoid breaking the script
return 1
fi
}
@@ -134,21 +134,42 @@ get_feature_paths() {
fi
# Use prefix-based lookup to support multiple branches per spec
local feature_dir=$(find_feature_dir_by_prefix "$repo_root" "$current_branch")
local feature_dir
if ! feature_dir=$(find_feature_dir_by_prefix "$repo_root" "$current_branch"); then
echo "ERROR: Failed to resolve feature directory" >&2
return 1
fi
cat <<EOF
REPO_ROOT='$repo_root'
CURRENT_BRANCH='$current_branch'
HAS_GIT='$has_git_repo'
FEATURE_DIR='$feature_dir'
FEATURE_SPEC='$feature_dir/spec.md'
IMPL_PLAN='$feature_dir/plan.md'
TASKS='$feature_dir/tasks.md'
RESEARCH='$feature_dir/research.md'
DATA_MODEL='$feature_dir/data-model.md'
QUICKSTART='$feature_dir/quickstart.md'
CONTRACTS_DIR='$feature_dir/contracts'
EOF
# Use printf '%q' to safely quote values, preventing shell injection
# via crafted branch names or paths containing special characters
printf 'REPO_ROOT=%q\n' "$repo_root"
printf 'CURRENT_BRANCH=%q\n' "$current_branch"
printf 'HAS_GIT=%q\n' "$has_git_repo"
printf 'FEATURE_DIR=%q\n' "$feature_dir"
printf 'FEATURE_SPEC=%q\n' "$feature_dir/spec.md"
printf 'IMPL_PLAN=%q\n' "$feature_dir/plan.md"
printf 'TASKS=%q\n' "$feature_dir/tasks.md"
printf 'RESEARCH=%q\n' "$feature_dir/research.md"
printf 'DATA_MODEL=%q\n' "$feature_dir/data-model.md"
printf 'QUICKSTART=%q\n' "$feature_dir/quickstart.md"
printf 'CONTRACTS_DIR=%q\n' "$feature_dir/contracts"
}
# Check if jq is available for safe JSON construction
has_jq() {
command -v jq >/dev/null 2>&1
}
# 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).
json_escape() {
local s="$1"
s="${s//\\/\\\\}"
s="${s//\"/\\\"}"
s="${s//$'\n'/\\n}"
s="${s//$'\t'/\\t}"
s="${s//$'\r'/\\r}"
printf '%s' "$s"
}
check_file() { [[ -f "$1" ]] && echo "$2" || echo "$2"; }