diff --git a/bash/bashadoc b/bash/bashadoc index 463e81d..7eba05a 100755 --- a/bash/bashadoc +++ b/bash/bashadoc @@ -33,9 +33,14 @@ esac # shellcheck disable=SC1090 source "$1" -package=$(grep "@package" "$1" | awk '{print $NF}' | tail -1) +# SUR-2829: anchor the @package extraction so it matches only literal +# `@package ` directives. The previous `grep "@package"` also +# matched `function @package() {` in doc.sh, producing `{` as the +# "package name". Anchoring on a leading-whitespace `@package ` and +# taking field 2 guarantees we read the directive's argument. +package=$(awk '/^[[:space:]]*@package[[:space:]]/{print $2}' "$1" | tail -1) funcs=$(declare -F | awk '{print $NF}' | grep -v "@") -if [ "$package" != "." ]; then +if [ -n "$package" ]; then title="\`$package\`" funcs=$(echo "$funcs" | grep "^$package::") else diff --git a/bash/copy-keys b/bash/copy-keys index 3be10e5..66d16e1 100755 --- a/bash/copy-keys +++ b/bash/copy-keys @@ -31,18 +31,27 @@ options::add -o n -d "namespace to query (default: current context)" -a -e NAMES options::parse "$@" log::info "Copying keys from pods with label $LABEL" +# SUR-2827: mirror fetch-all-pod-logs by building ns_args once and forwarding +# it to every kubectl invocation. Before this fix, only the initial +# k8s::pod_names_for_label call honoured -n NAMESPACE; the downstream +# get-pod and cp calls silently ran against the current-context default +# namespace, which would copy the wrong (or no) key material. +ns_args=() +if [ -n "${NAMESPACE:-}" ]; then + ns_args=(-n "$NAMESPACE") +fi while IFS= read -r pod; do [[ -z "$pod" ]] && continue - node_name=$(k8s::ctl get pod "$pod" -o json | $(commands::use jq) -r '.spec.nodeName') + node_name=$(k8s::ctl get pod "${ns_args[@]}" "$pod" -o json | $(commands::use jq) -r '.spec.nodeName') log::info "Working on $node_name with pod $pod" mkdir -p "keys/$node_name" ln -snf "$node_name" "keys/$pod" - if ! exec::capture k8s::ctl cp "$pod:/etc/sawtooth/keys/validator.priv" \ + if ! exec::capture k8s::ctl cp "${ns_args[@]}" "$pod:/etc/sawtooth/keys/validator.priv" \ "keys/$node_name/validator.priv" -c validator; then log::error "kubectl cp failed for pod $pod (validator.priv)" exit 1 fi - if ! exec::capture k8s::ctl cp "$pod:/etc/sawtooth/keys/validator.pub" \ + if ! exec::capture k8s::ctl cp "${ns_args[@]}" "$pod:/etc/sawtooth/keys/validator.pub" \ "keys/$node_name/validator.pub" -c validator; then log::error "kubectl cp failed for pod $pod (validator.pub)" exit 1 diff --git a/bash/exec.sh b/bash/exec.sh index 7ebf4d4..9807eab 100644 --- a/bash/exec.sh +++ b/bash/exec.sh @@ -31,13 +31,16 @@ function exec::capture() { local logfile=${LOGFILE:-"exec.log"} local tmpout tmpout=$(mktemp) - trap 'rm -f "$tmpout"' RETURN + # SUR-2831: clean tmpout up inline. The previous `trap ... RETURN` + # installation persisted into the caller's scope (last-writer-wins), + # silently clobbering any caller-installed RETURN trap. # Stream to caller stdout in real-time; second tee captures into tmpout. "$@" 2>&1 | tee -a "$logfile" | tee "$tmpout" exit_code=${PIPESTATUS[0]} # exec_output is an intentional output variable (no local) — callers read it. # shellcheck disable=SC2034 exec_output=$(<"$tmpout") + rm -f "$tmpout" else # exec_output is an intentional output variable (no local) — callers read it. # shellcheck disable=SC2034 diff --git a/bash/git.sh b/bash/git.sh index 7d3964b..9bdeffa 100644 --- a/bash/git.sh +++ b/bash/git.sh @@ -55,10 +55,17 @@ function git::project_url() { @doc Return the bare project URL: https://github.com/owner/repo @doc No trailing /commit suffix. Emits empty string for non-GitHub remotes. local origin_url - origin_url=$(git remote -v | grep "^origin" | head -1) + # SUR-2828: resolve `origin` exactly. The previous `grep "^origin"` matched + # `origin-fork`, `origin-mirror`, `origin2`, etc., and `head -1` over the + # interleaved `git remote -v` output was non-deterministic when multiple + # remotes were configured. + origin_url=$(git config --get remote.origin.url 2>/dev/null) + if [ -z "$origin_url" ]; then + origin_url=$(git remote -v | awk '$1=="origin" && $3=="(fetch)" {print $2; exit}') + fi if echo "$origin_url" | grep -q github; then local slug - slug=$(echo "$origin_url" | awk '{print $2}') + slug=${origin_url} slug=${slug//.git/} slug=${slug//git@github.com:/} slug=${slug//https:\/\/github.com\//} diff --git a/bash/secret.sh b/bash/secret.sh index e0a621a..befffb6 100644 --- a/bash/secret.sh +++ b/bash/secret.sh @@ -196,7 +196,11 @@ function secret::_env_as_file { function secret::clear { @doc Clear secret temporary files. - if [ -n "${SECRET_TMPFILES[0]}" ]; then + # SUR-2830: use array length, not [0]. A sparse array (e.g. element 0 unset + # by `unset 'SECRET_TMPFILES[0]'`) would otherwise leave the populated + # tail-end tempfiles on disk with decrypted secret material at 0600. + if [ "${#SECRET_TMPFILES[@]}" -gt 0 ]; then rm -f "${SECRET_TMPFILES[@]}" + SECRET_TMPFILES=() fi } diff --git a/tests/bashadoc.bats b/tests/bashadoc.bats new file mode 100644 index 0000000..b54671e --- /dev/null +++ b/tests/bashadoc.bats @@ -0,0 +1,51 @@ +#!/usr/bin/env bats +# SUR-2829: bashadoc must anchor @package extraction. The previous +# `grep "@package"` matched `function @package() {` in doc.sh, yielding +# `{` as the "package" and shipping garbage markdown into doc-*.tar.gz. + +setup() { + load 'helpers.bash' + helpers::isolate_home + FIXTURE_DIR=$(mktemp -d) + BASHADOC="$REPO_ROOT/bash/bashadoc" +} + +teardown() { + rm -rf "$FIXTURE_DIR" +} + +@test "bashadoc renders a header from @package and lists pkg::* functions (SUR-2829)" { + cat >"$FIXTURE_DIR/mypkg.sh" <<'EOF' +source "$(dirname "${BASH_SOURCE[0]}")/../bash/includer.sh" +@include doc + +@package mypkg + +function mypkg::fn() { + @doc Does the thing. + return 0 +} +EOF + out=$(bash "$BASHADOC" "$FIXTURE_DIR/mypkg.sh") + [[ "$out" == *"# \`mypkg\` package"* ]] + [[ "$out" == *"## \`mypkg::fn\`"* ]] + [[ "$out" != *"\`{\`"* ]] +} + +@test "bashadoc falls back to filename for libraries without @package (SUR-2829)" { + cat >"$FIXTURE_DIR/nopkg.sh" <<'EOF' +function bare_fn() { + return 0 +} +EOF + out=$(bash "$BASHADOC" "$FIXTURE_DIR/nopkg.sh") + [[ "$out" == *"# $FIXTURE_DIR/nopkg.sh package"* ]] + [[ "$out" == *"## \`bare_fn\`"* ]] + [[ "$out" != *"\`{\`"* ]] +} + +@test "bashadoc handles doc.sh (which defines function @package) without emitting '{' (SUR-2829)" { + out=$(bash "$BASHADOC" "$REPO_ROOT/bash/doc.sh") + [[ "$out" != *"\`{\`"* ]] + [[ "$out" == *"package"* ]] +} diff --git a/tests/copy-keys.bats b/tests/copy-keys.bats index e5b6c0a..ed963e8 100644 --- a/tests/copy-keys.bats +++ b/tests/copy-keys.bats @@ -39,3 +39,33 @@ teardown() { run bash "$COPY_KEYS" -l app=foo [[ "$status" -ne 0 ]] } + +# SUR-2827: -n NAMESPACE was threaded only into the initial +# k8s::pod_names_for_label call. Every downstream `get pod` and `cp` +# invocation silently dropped the namespace, copying key material from +# the wrong pod (or failing to find one). +@test "copy-keys forwards -n NAMESPACE to every kubectl call (SUR-2827)" { + run bash "$COPY_KEYS" -l app=foo -n my-ns + [[ "$status" -eq 0 ]] + # Every recorded kubectl invocation must carry `-n my-ns`. The recording + # stub appends "$@" one line per call. + while IFS= read -r line; do + [[ -z "$line" ]] && continue + [[ "$line" == *"-n my-ns"* ]] || { + echo "missing -n my-ns in: $line" + return 1 + } + done <"$KUBECTL_ARGV_LOG" + # Sanity-check we actually recorded the downstream `get pod` and `cp` + # calls, not just the initial pod list lookup. + grep -q "^get pod " "$KUBECTL_ARGV_LOG" + grep -q "^cp " "$KUBECTL_ARGV_LOG" +} + +@test "copy-keys passes no -n flag when NAMESPACE is unset (SUR-2827)" { + run bash "$COPY_KEYS" -l app=foo + [[ "$status" -eq 0 ]] + # No invocation should contain `-n ` (with trailing space) since the + # stub never receives a namespace argument. + run ! grep -q -- " -n " "$KUBECTL_ARGV_LOG" +} diff --git a/tests/exec.bats b/tests/exec.bats index 524876f..fd9f414 100644 --- a/tests/exec.bats +++ b/tests/exec.bats @@ -165,6 +165,41 @@ setup() { [ "$output" = "hello" ] } +# SUR-2831: exec::capture must not install a RETURN trap that leaks into +# the caller's scope. The previous `trap 'rm -f "$tmpout"' RETURN` was +# never cleared, occupied the trap slot permanently, and overwrote any +# caller-installed RETURN trap (last-writer-wins). + +@test "exec::capture leaves no RETURN trap installed in caller scope (SUR-2831)" { + out=$(bash -c " + LOGFILE='\$HOME/exec.log' + source '$REPO_ROOT/bash/includer.sh' + @include exec + caller() { + exec::capture true >/dev/null + trap -p RETURN + } + caller + ") + [ -z "$out" ] +} + +@test "exec::capture does not clobber a caller-installed RETURN trap (SUR-2831)" { + SENTINEL="$BATS_TEST_TMPDIR/return-sentinel" + rm -f "$SENTINEL" + bash -c " + LOGFILE='\$HOME/exec.log' + source '$REPO_ROOT/bash/includer.sh' + @include exec + caller() { + trap 'touch \"$SENTINEL\"' RETURN + exec::capture true >/dev/null + } + caller + " + [ -f "$SENTINEL" ] +} + @test "exec::capture populates exec_output via the tee branch" { run bash -c " LOGFILE='$HOME/exec-tee-output.log' diff --git a/tests/git.bats b/tests/git.bats index 877db2a..be19558 100644 --- a/tests/git.bats +++ b/tests/git.bats @@ -84,6 +84,22 @@ run_projecturl_with_remote() { [ "$out" = "$EXPECTED" ] } +# SUR-2828: git::project_url must resolve `origin` exactly, not any remote +# whose name starts with "origin" (e.g. origin-fork). + +@test "git::project_url ignores origin-fork remote (SUR-2828)" { + out=$(bash -c " + cd '$REPO' && + git remote remove origin 2>/dev/null + git remote add origin 'git@github.com:scealiontach/shell-scripts.git' + git remote add origin-fork 'git@github.com:someoneelse/shell-scripts-fork.git' + source '$REPO_ROOT/bash/includer.sh' + @include git + git::project_url + ") + [ "$out" = "https://github.com/scealiontach/shell-scripts" ] +} + # SUR-2454: git::tagsinhistory replaced with git for-each-ref @test "git::tagsinhistory returns one tag per line in newest-first order (SUR-2454)" { diff --git a/tests/secret.bats b/tests/secret.bats index 61e9bce..9040d6e 100644 --- a/tests/secret.bats +++ b/tests/secret.bats @@ -143,6 +143,28 @@ setup() { [ "$out" = "quoted-trap-fired" ] } +# SUR-2830: secret::clear must use array length, not [0]. A sparse array +# (element 0 unset, later indices populated) previously made the guard +# fail and the cleanup silently no-op, leaving 0600 tempfiles with secret +# material under $TMPDIR. +@test "secret::clear removes tempfiles when SECRET_TMPFILES[0] is unset (SUR-2830)" { + PROBE_DIR="$BATS_TEST_TMPDIR/sparse" + mkdir -p "$PROBE_DIR" + a="$PROBE_DIR/a" + b="$PROBE_DIR/b" + : >"$a" + : >"$b" + bash -c " + source '$REPO_ROOT/bash/includer.sh' + @include secret + SECRET_TMPFILES=('$a' '$b') + unset 'SECRET_TMPFILES[0]' + secret::clear + " + [ ! -e "$a" ] || [ ! -e "$b" ] # at least the surviving index was removed + [ ! -e "$b" ] +} + # SUR-2324: idempotency — installing twice must not stack chained calls. @test "secret::_install_cleanup_trap is idempotent within a shell (SUR-2324)" { SENTINEL="$BATS_TEST_TMPDIR/idempotent-sentinel"