Session Notes — 2026-06-26

Context

  • Repo: Ninja-Shield-v1
  • Network: VPN & private / internal
  • Tools: ninja-pentest_v2.sh, netbox.sh, ninja.sh, inventar.sh, HTML dashboards

Code Review — Last Commit (32b3e17 last seeds)

Files Reviewed

ninja.sh, ninja-pentest_v2.sh, netbox.sh, netbox_internet.sh, inventar.sh,
netbox/crumb-001-seed/commit.sh, netbox/crumb-001-seed/internet.commit.sh,
netbox/testhost/internet.commit.sh, netbox/testnb/commit.sh,
index.html, ninja-dashboard.html, panda_guertel.html

Findings Summary: 11 total — 3 Critical · 4 Warning · 4 Info

Critical

# File Issue
CR-01 ninja-dashboard.html:1511,1531,1580 DOM XSS — terminal input into div.innerHTML without escaping
CR-02 index.html:916,922-927 Markdown renderer XSS — raw content into innerHTML, allows javascript: URIs
CR-03 netbox/crumb-001-seed/internet.commit.sh:7 jq --argjson dev 000 — leading zeros invalid JSON, IFID silently empty

Warnings

  • WR-01 ninja-pentest_v2.sh — SSH banner logged via echo -e without ANSI stripping
  • WR-02/03 inventar.sh — ls in for-loop, unquoted command substitution
  • WR-04 ninja-dashboard.html / panda_guertel.html — no SRI hashes on external scripts

What's clean

  • ninja.sh, netbox.sh, netbox_internet.sh — whitelist validation, jq for JSON, set -u, clean() for log output, dry-run default

ninja-pentest_v2.sh — Security Hardening (applied this session)

Changes Made

  • Added set -u
  • Added is_valid_hostname() and is_valid_port() validators (whitelist)
  • Replaced eval "HOSTS_$idx=..." with printf -v "HOSTS_$idx" '%s' "$value"
  • Replaced profile getters with Bash-3 compatible indirect expansion ${!var:-}
  • Rebuilt check_port, check_ssh_banner, check_ssl_cert — args passed directly or shell-quoted via printf '%q'
  • Removed blanket curl -k — added explicit --insecure / -k flag
  • Validated --delay and --add arguments

Verdict on fixes

All fixes are correct and justified even on VPN/internal network:

Fix Why justified
printf -v instead of eval --add takes CLI input; eval is never safe
TLS on by default Targets are public internet hosts — cert checking is the point
Input validation Cheap, clear intent
${!var:-} indirect expansion Correct Bash pattern, no eval risk

One remaining open item

WR-01 — SSH banner ANSI escape injection (low risk on internal, still worth 1 line):

# In check_ssh_banner — strip non-printable chars before logging
banner=$(... | LC_ALL=C tr -dc '[:print:]')

TODO

  • [ ] Fix CR-01: Replace innerHTML with textContent or escaping function in ninja-dashboard.html
  • [ ] Fix CR-02: Sanitize markdown renderer output in index.html
  • [ ] Fix CR-03: Change jq --argjson dev 000 to jq --argjson dev 0 in internet.commit.sh
  • [ ] Fix WR-01: Add | LC_ALL=C tr -dc '[:print:]' to check_ssh_banner in ninja-pentest_v2.sh
  • [ ] Consider SRI hashes for external scripts in HTML dashboards

Learnings

  • printf -v varname '%s' value is the safe Bash alternative to eval "varname=value" for dynamic variable assignment
  • ${!varname} indirect expansion works in Bash 3.x (macOS default) — no eval needed for getters
  • TLS verification default matters even on internal networks when targets are public hosts
  • echo -e + unfiltered external input = ANSI escape injection risk in terminal logs
  • jq --argjson requires valid JSON — 000 is illegal (leading zeros), use 0
  • DOM XSS via innerHTML is the most common and highest-impact issue in internal dashboards