security: [LOW] Unsafe rm -rf in e2e.sh final_cleanup with fallback logic #3194
Description
Finding
File: sh/e2e/e2e.sh
Lines: 580-596
Severity: LOW
Description
The final_cleanup() function uses rm -rf with path validation, but the fallback path check uses pattern matching that could be bypassed:
case "${LOG_DIR}" in
"${SAFE_TMP_ROOT}"/spawn-e2e.*)
rm -rf "${LOG_DIR}"
;;
*)
log_warn "Refusing to rm -rf unexpected path: ${LOG_DIR}"
;;
esacImpact
- The pattern
spawn-e2e.*could match unexpected paths ifSAFE_TMP_ROOTis manipulated - Symlink attacks: if
LOG_DIRis a symlink to a sensitive directory,rm -rffollows it - Limited impact due to ownership check and pattern validation, but still a defense-in-depth gap
Recommendation
- Verify LOG_DIR ownership before deletion:
if [ -O "${LOG_DIR}" ]; then
rm -rf "${LOG_DIR}"
else
log_warn "LOG_DIR not owned by current user, refusing deletion"
fi- Use realpath to resolve symlinks before validation:
LOG_DIR_REAL=$(realpath "${LOG_DIR}" 2>/dev/null || echo "${LOG_DIR}")
case "${LOG_DIR_REAL}" in
"${SAFE_TMP_ROOT}"/spawn-e2e.*)
rm -rf "${LOG_DIR_REAL}"
;;
esac- Add bounds check:
# Refuse if LOG_DIR has too many path components (potential traversal)
if [ "$(echo "${LOG_DIR}" | tr '/' '\n' | wc -l)" -gt 5 ]; then
log_warn "LOG_DIR path too deep, refusing deletion"
exit 1
fi-- security/shell-scanner