T09 · Insecure Skill Coding Practices
Error
- Location
scripts/review.sh:359- Finding
Security findings are discarded because pipeline loops execute in subshells
- Content
View full analysis
/dev/null | head -5 | while IFS=: read -r line_num content; do add_issue "critical" "security" "SEC-001" "Possible hardcoded secret detected" "$file" "$line_num" done; then true; fi done <<< "$CHANGED_FILES" # SEC-002: SQL injection patterns if [[ "$HAS_PYTHON" == "true" ]] || [[ "$HAS_JS" == "true" ]]; then while IFS= read -r file; do if grep -nE '(execute|query)\s*\(\s*["\x27].*(%s|\$\{|" *\+)' "$file" 2>/dev/null | head -5 | while IFS=: read -r line_num content; do add_issue "critical" "security" "SEC-002" "Possible SQL injection via string interpolation" "$file" "$line_num" done; then true; fi done <<< "$CHANGED_FILES" fi # SEC-003: eval/exec usage while IFS= read -r file; do if grep -nE '\b(eval|exec)\s*\(' "$file" 2>/dev/null | head -5 | while IFS=: read -r line_num content; do add_issue "warning" "security" "SEC-003" "Use of eval/exec detected" "$file" "$line_num" done; then true; fi done <<< "$CHANGED_FILES" # SEC-004: HTTP URLs (should be HTTPS) while IFS= read -r file; do if grep -nE 'http://[^l][^o][^c]' "$file" 2>/dev/null | grep -v 'localhost\|127\.0\.0\.1\|0\.0\.0\.0' | head -5 | while IFS=: read -r line_num content; do add_issue "warning" "security" "SEC-004" "HTTP URL detected (should use HTTPS)" "$file" "$line_num" done; then true; fi done <<< "$CHANGED_FILES" # SEC-005: Disabled TLS verification while IFS= read -r file; do if grep -nEi '(verify\s*=\s*False|NODE_TLS_REJECT_UNAUTHORIZED.*0|InsecureSkipVerify.*true)' "$file" 2>/dev/null | head -5 | while IFS=: read -r line_num ...[truncated 3488 chars]- Remediation
View remediation
/dev/null | head -5 ) ``` Apply the same structure to SEC-002 through SEC-006 and to any other check that modifies arrays or counters from a pipeline-fed loop. Additional hardening should include: 1. Add automated tests containing one positive fixture for every security check. 2. Assert that each fixture appears in Markdown, JSON, and text output. 3. Assert that every critical fixture causes exit status 1. 4. Add a negative fixture for each check to detect false positives. 5. Run the test suite under the minimum documented Bash version and current supported versions. 6. Avoid relying solely on `shopt -s lastpipe`, because its behavior depends on shell mode and environment; process substitution is explicit and more portable within the declared Bash requirement. 7. Add a CI regression test that fails if a detected issue does not increment the corresponding parent-shell counter. ]]>