Skip to content

Commit 46bc65b

Browse files
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)
1 parent 017e1c4 commit 46bc65b

File tree

5 files changed

+180
-151
lines changed

5 files changed

+180
-151
lines changed

scripts/bash/check-prerequisites.sh

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -79,15 +79,28 @@ SCRIPT_DIR="$(CDPATH="" cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
7979
source "$SCRIPT_DIR/common.sh"
8080

8181
# Get feature paths and validate branch
82-
eval $(get_feature_paths)
82+
_paths_output=$(get_feature_paths) || { echo "ERROR: Failed to resolve feature paths" >&2; exit 1; }
83+
eval "$_paths_output"
84+
unset _paths_output
8385
check_feature_branch "$CURRENT_BRANCH" "$HAS_GIT" || exit 1
8486

8587
# If paths-only mode, output paths and exit (support JSON + paths-only combined)
8688
if $PATHS_ONLY; then
8789
if $JSON_MODE; then
8890
# Minimal JSON paths payload (no validation performed)
89-
printf '{"REPO_ROOT":"%s","BRANCH":"%s","FEATURE_DIR":"%s","FEATURE_SPEC":"%s","IMPL_PLAN":"%s","TASKS":"%s"}\n' \
90-
"$REPO_ROOT" "$CURRENT_BRANCH" "$FEATURE_DIR" "$FEATURE_SPEC" "$IMPL_PLAN" "$TASKS"
91+
if has_jq; then
92+
jq -cn \
93+
--arg repo_root "$REPO_ROOT" \
94+
--arg branch "$CURRENT_BRANCH" \
95+
--arg feature_dir "$FEATURE_DIR" \
96+
--arg feature_spec "$FEATURE_SPEC" \
97+
--arg impl_plan "$IMPL_PLAN" \
98+
--arg tasks "$TASKS" \
99+
'{REPO_ROOT:$repo_root,BRANCH:$branch,FEATURE_DIR:$feature_dir,FEATURE_SPEC:$feature_spec,IMPL_PLAN:$impl_plan,TASKS:$tasks}'
100+
else
101+
printf '{"REPO_ROOT":"%s","BRANCH":"%s","FEATURE_DIR":"%s","FEATURE_SPEC":"%s","IMPL_PLAN":"%s","TASKS":"%s"}\n' \
102+
"$(json_escape "$REPO_ROOT")" "$(json_escape "$CURRENT_BRANCH")" "$(json_escape "$FEATURE_DIR")" "$(json_escape "$FEATURE_SPEC")" "$(json_escape "$IMPL_PLAN")" "$(json_escape "$TASKS")"
103+
fi
91104
else
92105
echo "REPO_ROOT: $REPO_ROOT"
93106
echo "BRANCH: $CURRENT_BRANCH"
@@ -141,14 +154,25 @@ fi
141154
# Output results
142155
if $JSON_MODE; then
143156
# Build JSON array of documents
144-
if [[ ${#docs[@]} -eq 0 ]]; then
145-
json_docs="[]"
157+
if has_jq; then
158+
if [[ ${#docs[@]} -eq 0 ]]; then
159+
json_docs="[]"
160+
else
161+
json_docs=$(printf '%s\n' "${docs[@]}" | jq -R . | jq -s .)
162+
fi
163+
jq -cn \
164+
--arg feature_dir "$FEATURE_DIR" \
165+
--argjson docs "$json_docs" \
166+
'{FEATURE_DIR:$feature_dir,AVAILABLE_DOCS:$docs}'
146167
else
147-
json_docs=$(printf '"%s",' "${docs[@]}")
148-
json_docs="[${json_docs%,}]"
168+
if [[ ${#docs[@]} -eq 0 ]]; then
169+
json_docs="[]"
170+
else
171+
json_docs=$(printf '"%s",' "${docs[@]}")
172+
json_docs="[${json_docs%,}]"
173+
fi
174+
printf '{"FEATURE_DIR":"%s","AVAILABLE_DOCS":%s}\n' "$(json_escape "$FEATURE_DIR")" "$json_docs"
149175
fi
150-
151-
printf '{"FEATURE_DIR":"%s","AVAILABLE_DOCS":%s}\n' "$FEATURE_DIR" "$json_docs"
152176
else
153177
# Text output
154178
echo "FEATURE_DIR:$FEATURE_DIR"

scripts/bash/common.sh

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ find_feature_dir_by_prefix() {
120120
# Multiple matches - this shouldn't happen with proper naming convention
121121
echo "ERROR: Multiple spec directories found with prefix '$prefix': ${matches[*]}" >&2
122122
echo "Please ensure only one spec directory exists per numeric prefix." >&2
123-
echo "$specs_dir/$branch_name" # Return something to avoid breaking the script
123+
return 1
124124
fi
125125
}
126126

@@ -134,21 +134,42 @@ get_feature_paths() {
134134
fi
135135

136136
# Use prefix-based lookup to support multiple branches per spec
137-
local feature_dir=$(find_feature_dir_by_prefix "$repo_root" "$current_branch")
138-
139-
cat <<EOF
140-
REPO_ROOT='$repo_root'
141-
CURRENT_BRANCH='$current_branch'
142-
HAS_GIT='$has_git_repo'
143-
FEATURE_DIR='$feature_dir'
144-
FEATURE_SPEC='$feature_dir/spec.md'
145-
IMPL_PLAN='$feature_dir/plan.md'
146-
TASKS='$feature_dir/tasks.md'
147-
RESEARCH='$feature_dir/research.md'
148-
DATA_MODEL='$feature_dir/data-model.md'
149-
QUICKSTART='$feature_dir/quickstart.md'
150-
CONTRACTS_DIR='$feature_dir/contracts'
151-
EOF
137+
local feature_dir
138+
if ! feature_dir=$(find_feature_dir_by_prefix "$repo_root" "$current_branch"); then
139+
echo "ERROR: Failed to resolve feature directory" >&2
140+
return 1
141+
fi
142+
143+
# Use printf '%q' to safely quote values, preventing shell injection
144+
# via crafted branch names or paths containing special characters
145+
printf 'REPO_ROOT=%q\n' "$repo_root"
146+
printf 'CURRENT_BRANCH=%q\n' "$current_branch"
147+
printf 'HAS_GIT=%q\n' "$has_git_repo"
148+
printf 'FEATURE_DIR=%q\n' "$feature_dir"
149+
printf 'FEATURE_SPEC=%q\n' "$feature_dir/spec.md"
150+
printf 'IMPL_PLAN=%q\n' "$feature_dir/plan.md"
151+
printf 'TASKS=%q\n' "$feature_dir/tasks.md"
152+
printf 'RESEARCH=%q\n' "$feature_dir/research.md"
153+
printf 'DATA_MODEL=%q\n' "$feature_dir/data-model.md"
154+
printf 'QUICKSTART=%q\n' "$feature_dir/quickstart.md"
155+
printf 'CONTRACTS_DIR=%q\n' "$feature_dir/contracts"
156+
}
157+
158+
# Check if jq is available for safe JSON construction
159+
has_jq() {
160+
command -v jq >/dev/null 2>&1
161+
}
162+
163+
# 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).
165+
json_escape() {
166+
local s="$1"
167+
s="${s//\\/\\\\}"
168+
s="${s//\"/\\\"}"
169+
s="${s//$'\n'/\\n}"
170+
s="${s//$'\t'/\\t}"
171+
s="${s//$'\r'/\\r}"
172+
printf '%s' "$s"
152173
}
153174

154175
check_file() { [[ -f "$1" ]] && echo "$2" || echo "$2"; }

scripts/bash/create-new-feature.sh

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,17 @@ 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+
165176
# Resolve repository root. Prefer git information when available, but fall back
166177
# to searching for repository markers so the workflow still functions in repositories that
167178
# were initialised with --no-git.
@@ -300,14 +311,22 @@ TEMPLATE="$REPO_ROOT/.specify/templates/spec-template.md"
300311
SPEC_FILE="$FEATURE_DIR/spec.md"
301312
if [ -f "$TEMPLATE" ]; then cp "$TEMPLATE" "$SPEC_FILE"; else touch "$SPEC_FILE"; fi
302313

303-
# Set the SPECIFY_FEATURE environment variable for the current session
304-
export SPECIFY_FEATURE="$BRANCH_NAME"
314+
# Inform the user how to persist the feature variable in their own shell
315+
printf '# To persist: export SPECIFY_FEATURE=%q\n' "$BRANCH_NAME" >&2
305316

306317
if $JSON_MODE; then
307-
printf '{"BRANCH_NAME":"%s","SPEC_FILE":"%s","FEATURE_NUM":"%s"}\n' "$BRANCH_NAME" "$SPEC_FILE" "$FEATURE_NUM"
318+
if command -v jq >/dev/null 2>&1; then
319+
jq -cn \
320+
--arg branch_name "$BRANCH_NAME" \
321+
--arg spec_file "$SPEC_FILE" \
322+
--arg feature_num "$FEATURE_NUM" \
323+
'{BRANCH_NAME:$branch_name,SPEC_FILE:$spec_file,FEATURE_NUM:$feature_num}'
324+
else
325+
printf '{"BRANCH_NAME":"%s","SPEC_FILE":"%s","FEATURE_NUM":"%s"}\n' "$(json_escape "$BRANCH_NAME")" "$(json_escape "$SPEC_FILE")" "$(json_escape "$FEATURE_NUM")"
326+
fi
308327
else
309328
echo "BRANCH_NAME: $BRANCH_NAME"
310329
echo "SPEC_FILE: $SPEC_FILE"
311330
echo "FEATURE_NUM: $FEATURE_NUM"
312-
echo "SPECIFY_FEATURE environment variable set to: $BRANCH_NAME"
331+
printf '# To persist in your shell: export SPECIFY_FEATURE=%q\n' "$BRANCH_NAME"
313332
fi

scripts/bash/setup-plan.sh

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ SCRIPT_DIR="$(CDPATH="" cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
2828
source "$SCRIPT_DIR/common.sh"
2929

3030
# Get all paths and variables from common functions
31-
eval $(get_feature_paths)
31+
_paths_output=$(get_feature_paths) || { echo "ERROR: Failed to resolve feature paths" >&2; exit 1; }
32+
eval "$_paths_output"
33+
unset _paths_output
3234

3335
# Check if we're on a proper feature branch (only for git repos)
3436
check_feature_branch "$CURRENT_BRANCH" "$HAS_GIT" || exit 1
@@ -49,8 +51,18 @@ fi
4951

5052
# Output results
5153
if $JSON_MODE; then
52-
printf '{"FEATURE_SPEC":"%s","IMPL_PLAN":"%s","SPECS_DIR":"%s","BRANCH":"%s","HAS_GIT":"%s"}\n' \
53-
"$FEATURE_SPEC" "$IMPL_PLAN" "$FEATURE_DIR" "$CURRENT_BRANCH" "$HAS_GIT"
54+
if has_jq; then
55+
jq -cn \
56+
--arg feature_spec "$FEATURE_SPEC" \
57+
--arg impl_plan "$IMPL_PLAN" \
58+
--arg specs_dir "$FEATURE_DIR" \
59+
--arg branch "$CURRENT_BRANCH" \
60+
--arg has_git "$HAS_GIT" \
61+
'{FEATURE_SPEC:$feature_spec,IMPL_PLAN:$impl_plan,SPECS_DIR:$specs_dir,BRANCH:$branch,HAS_GIT:$has_git}'
62+
else
63+
printf '{"FEATURE_SPEC":"%s","IMPL_PLAN":"%s","SPECS_DIR":"%s","BRANCH":"%s","HAS_GIT":"%s"}\n' \
64+
"$(json_escape "$FEATURE_SPEC")" "$(json_escape "$IMPL_PLAN")" "$(json_escape "$FEATURE_DIR")" "$(json_escape "$CURRENT_BRANCH")" "$(json_escape "$HAS_GIT")"
65+
fi
5466
else
5567
echo "FEATURE_SPEC: $FEATURE_SPEC"
5668
echo "IMPL_PLAN: $IMPL_PLAN"

0 commit comments

Comments
 (0)