Skip to content

Commit 47b26de

Browse files
la14-1louisgv
andauthored
fix: harden Sprite exec against injection via org flags and grep patterns (#2446)
- Replace word-split _sprite_org_flags() call sites with _sprite_cmd() helper that uses a proper bash array for the -o flag, eliminating injection risk from org names with spaces or shell metacharacters - Validate _SPRITE_ORG against [A-Za-z0-9_-]+ in _sprite_validate_env - Use grep -qF (fixed-string) instead of grep -q for app name matching to prevent regex metacharacters in names from causing false matches - Use mktemp for _stderr_tmp in _sprite_exec instead of predictable PID-based path (/tmp/sprite-exec-err.$$) to prevent symlink attacks Closes #2436 Agent: complexity-hunter Co-authored-by: B <6723574+louisgv@users.noreply.github.com>
1 parent 9bf3c21 commit 47b26de

1 file changed

Lines changed: 31 additions & 15 deletions

File tree

sh/e2e/lib/clouds/sprite.sh

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,26 @@ set -eo pipefail
1616
# from concurrent sprite exec calls corrupting ~/.sprites/sprites.json.
1717
_SPRITE_ORG=""
1818

19-
# Helper: build org flags array for sprite CLI calls
19+
# Helper: build org flags for sprite CLI calls.
20+
# Outputs "-o" and the org name as separate lines for use with _sprite_cmd.
2021
_sprite_org_flags() {
2122
if [ -n "${_SPRITE_ORG}" ]; then
22-
printf '%s' "-o ${_SPRITE_ORG}"
23+
printf '%s\n%s' "-o" "${_SPRITE_ORG}"
2324
fi
2425
}
2526

27+
# Helper: run sprite CLI with org flags safely (no word-splitting).
28+
# Usage: _sprite_cmd [extra args...]
29+
# Reads org flags via _sprite_org_flags and builds a proper argument array.
30+
_sprite_cmd() {
31+
local _args
32+
_args=()
33+
if [ -n "${_SPRITE_ORG}" ]; then
34+
_args+=("-o" "${_SPRITE_ORG}")
35+
fi
36+
sprite "${_args[@]}" "$@"
37+
}
38+
2639
# Helper: fix corrupted sprite config (double-closing-brace from concurrent writes)
2740
_sprite_fix_config() {
2841
local cfg="${HOME}/.sprites/sprites.json"
@@ -93,6 +106,13 @@ _sprite_validate_env() {
93106
_SPRITE_ORG="${SPRITE_ORG:-}"
94107
fi
95108

109+
# Validate org name contains only safe characters (alphanumeric, dash, underscore)
110+
# to prevent injection via crafted org names in subsequent CLI calls.
111+
if [ -n "${_SPRITE_ORG}" ] && ! printf '%s' "${_SPRITE_ORG}" | grep -qE '^[A-Za-z0-9_-]+$'; then
112+
log_err "Invalid Sprite org name: ${_SPRITE_ORG}"
113+
return 1
114+
fi
115+
96116
if [ -n "${_SPRITE_ORG}" ]; then
97117
log_ok "Sprite credentials validated (org: ${_SPRITE_ORG})"
98118
else
@@ -134,15 +154,14 @@ _sprite_provision_verify() {
134154
# Check instance exists in sprite list
135155
_sprite_fix_config
136156
local sprite_output
137-
# shellcheck disable=SC2046
138-
sprite_output=$(sprite $(_sprite_org_flags) list 2>/dev/null || true)
157+
sprite_output=$(_sprite_cmd list 2>/dev/null || true)
139158

140159
if [ -z "${sprite_output}" ]; then
141160
log_err "Could not list Sprite instances"
142161
return 1
143162
fi
144163

145-
if ! printf '%s' "${sprite_output}" | grep -q "${app}"; then
164+
if ! printf '%s' "${sprite_output}" | grep -qF "${app}"; then
146165
log_err "Sprite instance ${app} not found in sprite list"
147166
return 1
148167
fi
@@ -171,14 +190,14 @@ _sprite_exec() {
171190
local cmd="$2"
172191
local _attempt=0
173192
local _max=3
174-
local _stderr_tmp="/tmp/sprite-exec-err.$$"
193+
local _stderr_tmp
194+
_stderr_tmp=$(mktemp /tmp/sprite-exec-err.XXXXXX) || return 1
175195

176196
while [ "${_attempt}" -lt "${_max}" ]; do
177197
_sprite_fix_config
178198
# Pipe the command via stdin to avoid interpolating it into the remote
179199
# command string — eliminates shell injection risk.
180-
# shellcheck disable=SC2046
181-
printf '%s' "${cmd}" | sprite $(_sprite_org_flags) exec -s "${app}" -- bash 2>"${_stderr_tmp}"
200+
printf '%s' "${cmd}" | _sprite_cmd exec -s "${app}" -- bash 2>"${_stderr_tmp}"
182201
local _rc=$?
183202
if [ "${_rc}" -eq 0 ]; then
184203
rm -f "${_stderr_tmp}"
@@ -208,18 +227,16 @@ _sprite_teardown() {
208227

209228
log_step "Tearing down ${app}..."
210229

211-
# shellcheck disable=SC2046
212-
sprite $(_sprite_org_flags) destroy --force "${app}" >/dev/null 2>&1 || true
230+
_sprite_cmd destroy --force "${app}" >/dev/null 2>&1 || true
213231

214232
# Brief wait for destruction to propagate
215233
sleep 2
216234

217235
# Verify deletion
218236
local sprite_output
219-
# shellcheck disable=SC2046
220-
sprite_output=$(sprite $(_sprite_org_flags) list 2>/dev/null || true)
237+
sprite_output=$(_sprite_cmd list 2>/dev/null || true)
221238

222-
if printf '%s' "${sprite_output}" | grep -q "${app}"; then
239+
if printf '%s' "${sprite_output}" | grep -qF "${app}"; then
223240
log_warn "Sprite instance ${app} may still exist"
224241
else
225242
log_ok "Sprite instance ${app} torn down"
@@ -241,8 +258,7 @@ _sprite_cleanup_stale() {
241258

242259
# List all sprites
243260
local sprite_output
244-
# shellcheck disable=SC2046
245-
sprite_output=$(sprite $(_sprite_org_flags) list 2>/dev/null || true)
261+
sprite_output=$(_sprite_cmd list 2>/dev/null || true)
246262

247263
if [ -z "${sprite_output}" ]; then
248264
log_info "Could not list Sprite instances or none found — skipping cleanup"

0 commit comments

Comments
 (0)