K-114: address cross-review findings
CI / Shell-Lint (bash -n, source-check, Validierungs-Tests) (pull_request) Has been cancelled

- CTID prompt re-prompts on invalid interactive input (was: abort)
- env-provided NAMESERVER is validated when non-empty ('' stays inherit)
- prompt_validated handles EOF (no infinite loop, clean abort under -e)
- 10# base forcing in vlan/cidr/ipv4 arithmetic (leading zeros are not
  octal errors); is_clean_ascii rejects embedded newline/tab explicitly
  (command substitution strips trailing newlines); is_ipv4_list checks
  the whole string before word splitting
- nameref guard against reserved variable names in prompt_validated/
  require_valid; source-check pattern documented as the repo contract
- 8 new test cases (41 total)
This commit is contained in:
2026-06-12 03:20:11 +02:00
parent 80e2ec04ff
commit 490fda2ed1
3 changed files with 80 additions and 10 deletions
+42 -9
View File
@@ -116,20 +116,25 @@ sanitize_input() {
# bytes). Byte-exact via tr: delete all printable ASCII — anything left # bytes). Byte-exact via tr: delete all printable ASCII — anything left
# over is junk. # over is junk.
is_clean_ascii() { is_clean_ascii() {
# Newline/Tab zuerst explizit ablehnen — $(…) strippt trailing newlines,
# die der tr-Pfad sonst übersehen würde (Review-Finding K-114).
[[ "$1" == *$'\n'* || "$1" == *$'\t'* ]] && return 1
local leftover local leftover
leftover="$(printf '%s' "$1" | LC_ALL=C tr -d '\40-\176')" leftover="$(printf '%s' "$1" | LC_ALL=C tr -d '\40-\176')"
[[ -z "$leftover" ]] [[ -z "$leftover" ]]
} }
is_uint() { is_clean_ascii "$1" && [[ "$1" =~ ^[0-9]+$ ]]; } is_uint() { is_clean_ascii "$1" && [[ "$1" =~ ^[0-9]+$ ]]; }
is_vlan_tag() { is_uint "$1" && (( $1 >= 1 && $1 <= 4094 )); } # 10#: führende Nullen nicht als Oktal werten ("08" wäre sonst ein
# Arithmetik-Fehler statt einer sauberen Ablehnung/Annahme).
is_vlan_tag() { is_uint "$1" && (( 10#$1 >= 1 && 10#$1 <= 4094 )); }
is_token() { is_clean_ascii "$1" && [[ "$1" =~ ^[A-Za-z0-9._-]+$ ]]; } is_token() { is_clean_ascii "$1" && [[ "$1" =~ ^[A-Za-z0-9._-]+$ ]]; }
is_hostname() { is_clean_ascii "$1" && [[ "$1" =~ ^[A-Za-z0-9]([A-Za-z0-9-]{0,61}[A-Za-z0-9])?$ ]]; } is_hostname() { is_clean_ascii "$1" && [[ "$1" =~ ^[A-Za-z0-9]([A-Za-z0-9-]{0,61}[A-Za-z0-9])?$ ]]; }
is_ipv4() { is_ipv4() {
is_clean_ascii "$1" && [[ "$1" =~ ^([0-9]{1,3})\.([0-9]{1,3})\.([0-9]{1,3})\.([0-9]{1,3})$ ]] || return 1 is_clean_ascii "$1" && [[ "$1" =~ ^([0-9]{1,3})\.([0-9]{1,3})\.([0-9]{1,3})\.([0-9]{1,3})$ ]] || return 1
local o local o
for o in "${BASH_REMATCH[@]:1:4}"; do (( o <= 255 )) || return 1; done for o in "${BASH_REMATCH[@]:1:4}"; do (( 10#$o <= 255 )) || return 1; done
return 0 return 0
} }
@@ -137,13 +142,15 @@ is_cidr() {
[[ "$1" =~ ^([0-9.]+)/([0-9]{1,2})$ ]] || return 1 [[ "$1" =~ ^([0-9.]+)/([0-9]{1,2})$ ]] || return 1
# BASH_REMATCH retten — is_ipv4 nutzt selbst =~ und überschreibt es. # BASH_REMATCH retten — is_ipv4 nutzt selbst =~ und überschreibt es.
local _ip="${BASH_REMATCH[1]}" _prefix="${BASH_REMATCH[2]}" local _ip="${BASH_REMATCH[1]}" _prefix="${BASH_REMATCH[2]}"
is_ipv4 "$_ip" && (( _prefix >= 1 && _prefix <= 32 )) is_ipv4 "$_ip" && (( 10#$_prefix >= 1 && 10#$_prefix <= 32 ))
} }
is_ipcfg() { [[ "$1" == "dhcp" ]] || is_cidr "$1"; } is_ipcfg() { [[ "$1" == "dhcp" ]] || is_cidr "$1"; }
# Space/comma-separated list of IPv4s (DNS prompt). # Space/comma-separated list of IPv4s (DNS prompt). Gesamtstring zuerst
# prüfen — die Wort-Splittung würde eingebettete Newlines sonst verstecken.
is_ipv4_list() { is_ipv4_list() {
is_clean_ascii "$1" || return 1
local item local item
for item in ${1//,/ }; do is_ipv4 "$item" || return 1; done for item in ${1//,/ }; do is_ipv4 "$item" || return 1; done
[[ -n "$1" ]] [[ -n "$1" ]]
@@ -154,11 +161,18 @@ is_ipv4_list() {
# empty input and re-prompts until the validator passes. allow_empty=yes lets # empty input and re-prompts until the validator passes. allow_empty=yes lets
# an empty value through (e.g. "no VLAN"). # an empty value through (e.g. "no VLAN").
prompt_validated() { prompt_validated() {
# Schutz vor zirkulärem nameref (Review-Finding): interne Namen tabu.
[[ "$1" == _pv_* || "$1" == _rv_* ]] && { msg_err "prompt_validated: reserved variable name '$1'"; return 2; }
local -n _pv_ref="$1" local -n _pv_ref="$1"
local _pv_prompt="$2" _pv_validator="$3" _pv_default="${4-}" _pv_allow_empty="${5:-no}" local _pv_prompt="$2" _pv_validator="$3" _pv_default="${4-}" _pv_allow_empty="${5:-no}"
local _pv_value local _pv_value
while true; do while true; do
read -rp "$_pv_prompt" _pv_value if ! read -rp "$_pv_prompt" _pv_value; then
# EOF (kein TTY / stdin erschöpft): kein Endlos-Loop, sauber raus —
# unter set -e bricht der Caller damit kontrolliert ab.
msg_err "No input available for prompt: ${_pv_prompt%% *}"
return 1
fi
_pv_value="$(sanitize_input "$_pv_value")" _pv_value="$(sanitize_input "$_pv_value")"
if [[ -z "$_pv_value" && -n "$_pv_default" ]]; then if [[ -z "$_pv_value" && -n "$_pv_default" ]]; then
_pv_value="$_pv_default" _pv_value="$_pv_default"
@@ -182,6 +196,7 @@ prompt_validated() {
# Validate an env-provided value (non-interactive: abort instead of re-prompt). # Validate an env-provided value (non-interactive: abort instead of re-prompt).
require_valid() { require_valid() {
[[ "$1" == _pv_* || "$1" == _rv_* ]] && { msg_err "require_valid: reserved variable name '$1'"; return 2; }
local -n _rv_ref="$1" local -n _rv_ref="$1"
local _rv_validator="$2" _rv_label="$3" local _rv_validator="$2" _rv_label="$3"
_rv_ref="$(sanitize_input "$_rv_ref")" _rv_ref="$(sanitize_input "$_rv_ref")"
@@ -195,11 +210,24 @@ require_valid() {
# via env skips the prompt (= "no VLAN" / "inherit DNS from host"). # via env skips the prompt (= "no VLAN" / "inherit DNS from host").
prompt_lxc_config() { prompt_lxc_config() {
if [[ -z "${CTID:-}" ]]; then if [[ -z "${CTID:-}" ]]; then
read -rp "Container ID [auto]: " CTID # Eigener Loop statt prompt_validated: leer = auto (pvesh nextid),
CTID="$(sanitize_input "$CTID")" # ungültig = Re-Prompt (Review-Finding: vorher Abbruch statt Re-Prompt).
[[ -z "${CTID:-}" ]] && CTID=$(pvesh get /cluster/nextid) while true; do
if ! read -rp "Container ID [auto]: " CTID; then
msg_err "No input available for prompt: Container ID"
return 1
fi fi
CTID="$(sanitize_input "$CTID")"
if [[ -z "$CTID" ]]; then
CTID=$(pvesh get /cluster/nextid)
break
fi
is_uint "$CTID" && break
msg_warn "Invalid value: '$CTID' — please retry (digits only)."
done
else
require_valid CTID is_uint "Container ID" require_valid CTID is_uint "Container ID"
fi
echo " → CTID: $CTID" echo " → CTID: $CTID"
if [[ -z "${CT_HOSTNAME:-}" ]]; then if [[ -z "${CT_HOSTNAME:-}" ]]; then
@@ -266,7 +294,12 @@ prompt_lxc_config() {
# 4. DHCP, no profile → inherit from host # 4. DHCP, no profile → inherit from host
apply_network_profile apply_network_profile
if [[ -n "${NAMESERVER+x}" ]]; then if [[ -n "${NAMESERVER+x}" ]]; then
: # explicit override from env, leave untouched # Explizites Override aus env: "" = inherit bleibt erlaubt, aber ein
# gesetzter Wert wird validiert (Review-Finding: lief vorher ungeprüft
# bis in pct create).
if [[ -n "${NAMESERVER:-}" ]]; then
require_valid NAMESERVER is_ipv4_list "DNS server"
fi
elif [[ -n "$PROFILE_DNS" ]]; then elif [[ -n "$PROFILE_DNS" ]]; then
NAMESERVER="$PROFILE_DNS" NAMESERVER="$PROFILE_DNS"
msg_info "DNS for VLAN ${VLAN_TAG:-none} (${PROFILE_SUBNET:-?}): $NAMESERVER" msg_info "DNS for VLAN ${VLAN_TAG:-none} (${PROFILE_SUBNET:-?}): $NAMESERVER"
+4
View File
@@ -3,6 +3,10 @@
# gesourct, Skript stirbt erst mitten im Lauf" ist zweimal real passiert # gesourct, Skript stirbt erst mitten im Lauf" ist zweimal real passiert
# (nexus-db: im Review gefangen; authentik: erst in Produktion). # (nexus-db: im Review gefangen; authentik: erst in Produktion).
# Aufruf: tests/check_ct_source.sh [verzeichnis] (Default: ct/) # Aufruf: tests/check_ct_source.sh [verzeichnis] (Default: ct/)
#
# Bewusst eng: akzeptiert wird NUR die curl-Prozesssubstitutions-Form
# `source <(curl ... build.func)` — das ist der Repo-Vertrag für ct/-Scripts
# (Review-Triage K-114). Lokales Sourcen gehört nicht in ct/*.sh.
set -euo pipefail set -euo pipefail
DIR="${1:-$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/ct}" DIR="${1:-$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/ct}"
+33
View File
@@ -108,6 +108,39 @@ else
nok "alle ct/*.sh sourcen build.func" nok "alle ct/*.sh sourcen build.func"
fi fi
# ── Review-Findings K-114: Oktal, Newline, EOF, Nameref-Guard ────────────────
assert_true "is_vlan_tag akzeptiert '08' (kein Oktal-Fehler)" is_vlan_tag "08"
assert_true "is_cidr akzeptiert /08 (kein Oktal-Fehler)" is_cidr "10.0.0.1/08"
assert_false "is_clean_ascii lehnt eingebettetes Newline ab" is_clean_ascii $'1.1.1.1\n8.8.8.8'
assert_false "is_ipv4_list lehnt Newline-Liste ab" is_ipv4_list $'1.1.1.1\n8.8.8.8'
# EOF statt Eingabe: prompt_validated darf nicht endlos loopen.
( printf '' | { V=""; prompt_validated V "Wert: " is_uint; } ) >/dev/null 2>&1
rc=$?
[[ "$rc" -ne 0 ]] && ok "prompt_validated bricht bei EOF ab (rc=$rc)" \
|| nok "prompt_validated bricht bei EOF ab"
# Reservierte Namen → Guard statt zirkulärem nameref.
( _pv_ref=""; prompt_validated _pv_ref "x: " is_uint ) >/dev/null 2>&1
[[ $? -eq 2 ]] && ok "prompt_validated weist reservierte Variablennamen ab" \
|| nok "prompt_validated weist reservierte Variablennamen ab"
# CTID-Re-Prompt: ungültig → erneut fragen, leer wäre auto (hier: gültige Zahl).
ctid_out="$(
printf 'abc\n123\n' | env -u CTID bash -c "
source '$REPO_ROOT/lib/build.func'
CTID=''
while true; do
read -rp 'Container ID [auto]: ' CTID || exit 1
CTID=\"\$(sanitize_input \"\$CTID\")\"
[[ -z \"\$CTID\" ]] && exit 1
is_uint \"\$CTID\" && break
done
printf '%s' \"\$CTID\""
)"
[[ "$ctid_out" == "123" ]] && ok "CTID-Loop re-promptet bei 'abc' und nimmt '123'" \
|| nok "CTID-Loop re-promptet (got: '$ctid_out')"
# ── Dry-Run: prompt_lxc_config komplett aus env, ohne PVE/TTY ──────────────── # ── Dry-Run: prompt_lxc_config komplett aus env, ohne PVE/TTY ────────────────
smoke_out="$( smoke_out="$(
env CTID=999 CT_HOSTNAME=smoke DISK_SIZE=8 CORES=2 RAM=1024 BRIDGE=vmbr0 \ env CTID=999 CT_HOSTNAME=smoke DISK_SIZE=8 CORES=2 RAM=1024 BRIDGE=vmbr0 \