[Sprint] sprint-loop-41

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 <name>` 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

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

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

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\//}

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
 }

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"* ]]
+}

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"
+}

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'

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)" {

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"