From dafc922e726b31eefc7e26aa741b4ac1e40b56c6 Mon Sep 17 00:00:00 2001
From: houseme <housemecn@gmail.com>
Date: Sat, 8 Aug 2026 21:32:29 +0800
Subject: [PATCH] chore: harden hotpath profiling artifact collection (#5848)

Co-authored-by: heihutu <heihutu@gmail.com>
---
 scripts/capture_remote_journal_errors.sh      | 107 ++++++++++++++++++
 scripts/collect_remote_samply_artifacts.sh    |  98 ++++++++++++++++
 scripts/run_samply_attach_window.sh           |   4 +-
 scripts/test_capture_remote_journal_errors.sh |  69 +++++++++++
 .../test_collect_remote_samply_artifacts.sh   |  80 +++++++++++++
 5 files changed, 357 insertions(+), 1 deletion(-)
 create mode 100755 scripts/capture_remote_journal_errors.sh
 create mode 100755 scripts/collect_remote_samply_artifacts.sh
 create mode 100755 scripts/test_capture_remote_journal_errors.sh
 create mode 100755 scripts/test_collect_remote_samply_artifacts.sh

diff --git a/scripts/capture_remote_journal_errors.sh b/scripts/capture_remote_journal_errors.sh
new file mode 100755
index 000000000..f8899b789
--- /dev/null
+++ b/scripts/capture_remote_journal_errors.sh
@@ -0,0 +1,107 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+usage() {
+  cat <<'USAGE'
+Usage: scripts/capture_remote_journal_errors.sh --nodes <csv> --since <iso-time> --label <label> --out-dir <dir> [options]
+
+Capture RustFS journal lines matching auth/error/failure patterns for a UTC
+validation window. The --since value is normalized to the journalctl-friendly
+"YYYY-MM-DD HH:MM:SS UTC" form before it is sent to remote nodes.
+
+Options:
+  --nodes <csv>          Comma-separated node names, for example vm004,vm005.
+  --since <iso-time>     UTC ISO timestamp, for example 2026-08-08T09:05:45Z.
+  --label <label>        Prefix used for output files.
+  --out-dir <dir>        Local output directory.
+  --unit <name>          systemd unit name. Default: rustfs.
+  --filter-regex <expr>  grep -Ei pattern. Default captures auth/signature/error/warn/fail/panic.
+  --ssh-bin <path>       SSH binary or test double. Default: ssh.
+  -h, --help             Show this help.
+USAGE
+}
+
+die() {
+  echo "error: $*" >&2
+  exit 2
+}
+
+shell_quote() {
+  local value=${1//\'/\'\\\'\'}
+  printf "'%s'" "$value"
+}
+
+validate_name() {
+  local field="$1"
+  local value="$2"
+  [[ "$value" =~ ^[A-Za-z0-9._@-]+$ ]] || die "$field contains unsafe characters: $value"
+}
+
+format_since_utc() {
+  python3 - "$1" <<'PY'
+from datetime import datetime, timezone
+import sys
+
+value = sys.argv[1].strip()
+if value.endswith("Z"):
+    value = value[:-1] + "+00:00"
+try:
+    parsed = datetime.fromisoformat(value)
+except ValueError as err:
+    raise SystemExit(f"invalid ISO timestamp: {err}")
+if parsed.tzinfo is None:
+    parsed = parsed.replace(tzinfo=timezone.utc)
+print(parsed.astimezone(timezone.utc).strftime("%Y-%m-%d %H:%M:%S UTC"))
+PY
+}
+
+NODES_CSV=""
+SINCE_ISO=""
+LABEL=""
+OUT_DIR=""
+UNIT="rustfs"
+FILTER_REGEX="No valid auth token|auth|signature|error|panic|fail|warn"
+SSH_BIN="${SSH_BIN:-ssh}"
+
+while [[ $# -gt 0 ]]; do
+  case "$1" in
+    --nodes) NODES_CSV="${2:-}"; shift 2 ;;
+    --since) SINCE_ISO="${2:-}"; shift 2 ;;
+    --label) LABEL="${2:-}"; shift 2 ;;
+    --out-dir) OUT_DIR="${2:-}"; shift 2 ;;
+    --unit) UNIT="${2:-}"; shift 2 ;;
+    --filter-regex) FILTER_REGEX="${2:-}"; shift 2 ;;
+    --ssh-bin) SSH_BIN="${2:-}"; shift 2 ;;
+    -h|--help) usage; exit 0 ;;
+    *) die "unknown argument: $1" ;;
+  esac
+done
+
+[[ -n "$NODES_CSV" ]] || die "--nodes is required"
+[[ -n "$SINCE_ISO" ]] || die "--since is required"
+[[ -n "$LABEL" ]] || die "--label is required"
+[[ -n "$OUT_DIR" ]] || die "--out-dir is required"
+[[ -n "$UNIT" ]] || die "--unit must not be empty"
+validate_name "--label" "$LABEL"
+validate_name "--unit" "$UNIT"
+
+since_journal=$(format_since_utc "$SINCE_ISO")
+mkdir -p "$OUT_DIR"
+
+IFS=',' read -r -a nodes <<<"$NODES_CSV"
+captured=0
+for node in "${nodes[@]}"; do
+  node="${node//[[:space:]]/}"
+  [[ -n "$node" ]] || continue
+  validate_name "node" "$node"
+
+  output_file="$OUT_DIR/${LABEL}-${node}-journal-errors.txt"
+  journal_cmd="journalctl -u $(shell_quote "$UNIT") --since $(shell_quote "$since_journal") --no-pager"
+  remote_cmd="sudo su - root -c $(shell_quote "$journal_cmd")"
+  "$SSH_BIN" "$node" "$remote_cmd" 2>&1 | grep -Ei "$FILTER_REGEX" >"$output_file" || true
+  captured=$((captured + 1))
+done
+
+[[ "$captured" -gt 0 ]] || die "--nodes did not contain any usable node names"
+echo "journal_since=$since_journal"
+echo "captured_nodes=$captured"
diff --git a/scripts/collect_remote_samply_artifacts.sh b/scripts/collect_remote_samply_artifacts.sh
new file mode 100755
index 000000000..e41ced734
--- /dev/null
+++ b/scripts/collect_remote_samply_artifacts.sh
@@ -0,0 +1,98 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+usage() {
+  cat <<'USAGE'
+Usage: scripts/collect_remote_samply_artifacts.sh --mapping <file> --out-dir <dir> [options]
+
+Copy samply profiles and symbol artifacts from RustFS nodes without allowing
+scp to consume the mapping loop's stdin.
+
+Mapping file format:
+  <node> <remote_artifact_dir>
+
+Example:
+  vm004 /data/rustfs/hotpath/20260808-put-1m
+  vm005 /data/rustfs/hotpath/20260808-put-1m
+
+Options:
+  --mapping <file>     Node and remote artifact directory pairs.
+  --out-dir <dir>      Local directory where node subdirectories are created.
+  --remote-root <dir>  Required parent path for remote artifact directories.
+                       Default: /data/rustfs.
+  --ssh-bin <path>     SSH binary or test double. Default: ssh.
+  --scp-bin <path>     SCP binary or test double. Default: scp.
+  -h, --help           Show this help.
+USAGE
+}
+
+die() {
+  echo "error: $*" >&2
+  exit 2
+}
+
+shell_quote() {
+  local value=${1//\'/\'\\\'\'}
+  printf "'%s'" "$value"
+}
+
+validate_node() {
+  [[ "$1" =~ ^[A-Za-z0-9._-]+$ ]] || die "node contains unsafe characters: $1"
+}
+
+validate_path() {
+  [[ "$1" =~ ^/[A-Za-z0-9._/@+=-]+$ ]] || die "path contains unsafe characters: $1"
+}
+
+MAPPING=""
+OUT_DIR=""
+REMOTE_ROOT="/data/rustfs"
+SSH_BIN="${SSH_BIN:-ssh}"
+SCP_BIN="${SCP_BIN:-scp}"
+
+while [[ $# -gt 0 ]]; do
+  case "$1" in
+    --mapping) MAPPING="${2:-}"; shift 2 ;;
+    --out-dir) OUT_DIR="${2:-}"; shift 2 ;;
+    --remote-root) REMOTE_ROOT="${2:-}"; shift 2 ;;
+    --ssh-bin) SSH_BIN="${2:-}"; shift 2 ;;
+    --scp-bin) SCP_BIN="${2:-}"; shift 2 ;;
+    -h|--help) usage; exit 0 ;;
+    *) die "unknown argument: $1" ;;
+  esac
+done
+
+[[ -n "$MAPPING" ]] || die "--mapping is required"
+[[ -f "$MAPPING" ]] || die "mapping file not found: $MAPPING"
+[[ -n "$OUT_DIR" ]] || die "--out-dir is required"
+[[ -n "$REMOTE_ROOT" ]] || die "--remote-root must not be empty"
+[[ "$REMOTE_ROOT" == /* ]] || die "--remote-root must be an absolute path"
+validate_path "$REMOTE_ROOT"
+
+REMOTE_ROOT="${REMOTE_ROOT%/}"
+mkdir -p "$OUT_DIR"
+
+processed=0
+while IFS= read -r line || [[ -n "$line" ]]; do
+  [[ -z "${line//[[:space:]]/}" ]] && continue
+  [[ "$line" =~ ^[[:space:]]*# ]] && continue
+
+  read -r node remote_dir extra <<<"$line"
+  [[ -n "${node:-}" && -n "${remote_dir:-}" && -z "${extra:-}" ]] || die "mapping lines must contain exactly two fields: $line"
+  validate_node "$node"
+  [[ "$remote_dir" == /* ]] || die "remote artifact dir must be absolute for $node: $remote_dir"
+  validate_path "$remote_dir"
+  [[ "$remote_dir" == "$REMOTE_ROOT"/* ]] || die "remote artifact dir must be under $REMOTE_ROOT for $node: $remote_dir"
+
+  node_out_dir="$OUT_DIR/$node"
+  mkdir -p "$node_out_dir"
+
+  quoted_remote_dir=$(shell_quote "$remote_dir")
+  remote_cmd="chmod -R a+rX $quoted_remote_dir; find $quoted_remote_dir -maxdepth 1 -type f -printf '%f %s bytes\n'"
+  "$SSH_BIN" "$node" "sudo su - root -c $(shell_quote "$remote_cmd")" >"$OUT_DIR/${node}-files.txt" 2>&1
+  "$SCP_BIN" -q -r "$node:${remote_dir%/}/"* "$node_out_dir/" </dev/null 2>"$OUT_DIR/${node}-scp.err"
+  processed=$((processed + 1))
+done <"$MAPPING"
+
+[[ "$processed" -gt 0 ]] || die "mapping file did not contain any nodes"
+echo "collected_nodes=$processed"
diff --git a/scripts/run_samply_attach_window.sh b/scripts/run_samply_attach_window.sh
index e281e3fbb..c46285734 100755
--- a/scripts/run_samply_attach_window.sh
+++ b/scripts/run_samply_attach_window.sh
@@ -12,7 +12,9 @@ usage() {
 Usage: scripts/run_samply_attach_window.sh --pid <pid> --duration-secs <n> --output <profile.json.gz> [options]
 
 Attach samply to an already-running process for a bounded window and force a
-Ctrl+C-style shutdown so samply writes the profile artifact.
+Ctrl+C-style shutdown so samply writes the profile artifact. This script uses
+direct `samply record -p`; `cargo samply` launches a cargo target and is not
+suitable for attaching to an existing RustFS service PID.
 
 Options:
   --pid <pid>              Existing process id to profile.
diff --git a/scripts/test_capture_remote_journal_errors.sh b/scripts/test_capture_remote_journal_errors.sh
new file mode 100755
index 000000000..cf431888f
--- /dev/null
+++ b/scripts/test_capture_remote_journal_errors.sh
@@ -0,0 +1,69 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+repo_root=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)
+tmp_dir=$(mktemp -d)
+trap 'rm -rf "$tmp_dir"' EXIT
+
+mock_bin="$tmp_dir/bin"
+mkdir -p "$mock_bin"
+
+cat >"$mock_bin/ssh" <<'EOF'
+#!/usr/bin/env bash
+set -euo pipefail
+printf 'ssh:%s\n' "$*" >>"$CALL_LOG"
+if [[ "${MOCK_JOURNAL_MODE:-}" == "clean" ]]; then
+  echo "rustfs request completed"
+  exit 0
+fi
+echo "rustfs request completed"
+echo "No valid auth token"
+echo "WARN replay cache overflow"
+EOF
+
+chmod +x "$mock_bin/ssh"
+
+export CALL_LOG="$tmp_dir/calls.log"
+output=$("$repo_root/scripts/capture_remote_journal_errors.sh" \
+  --nodes "vm004,vm005" \
+  --since "2026-08-08T09:05:45Z" \
+  --label "get-1m" \
+  --out-dir "$tmp_dir/out" \
+  --ssh-bin "$mock_bin/ssh")
+
+grep -q 'journal_since=2026-08-08 09:05:45 UTC' <<<"$output"
+grep -q 'captured_nodes=2' <<<"$output"
+grep -q '2026-08-08 09:05:45 UTC' "$CALL_LOG"
+grep -q 'sudo su - root -c' "$CALL_LOG"
+if grep -q '2026-08-08T09:05:45Z' "$CALL_LOG"; then
+  echo "raw ISO timestamp was sent to journalctl" >&2
+  exit 1
+fi
+grep -q 'No valid auth token' "$tmp_dir/out/get-1m-vm004-journal-errors.txt"
+grep -q 'WARN replay cache overflow' "$tmp_dir/out/get-1m-vm005-journal-errors.txt"
+if grep -q 'rustfs request completed' "$tmp_dir/out/get-1m-vm004-journal-errors.txt"; then
+  echo "non-error journal line was not filtered out" >&2
+  exit 1
+fi
+
+export MOCK_JOURNAL_MODE=clean
+"$repo_root/scripts/capture_remote_journal_errors.sh" \
+  --nodes "vm004" \
+  --since "2026-08-08T09:05:45Z" \
+  --label "clean" \
+  --out-dir "$tmp_dir/clean-out" \
+  --ssh-bin "$mock_bin/ssh" >"$tmp_dir/clean.stdout"
+[[ ! -s "$tmp_dir/clean-out/clean-vm004-journal-errors.txt" ]]
+
+if "$repo_root/scripts/capture_remote_journal_errors.sh" \
+  --nodes "vm004" \
+  --since "2026-08-08T09:05:45Z" \
+  --label "../escape" \
+  --out-dir "$tmp_dir/unsafe-out" \
+  --ssh-bin "$mock_bin/ssh" >"$tmp_dir/unsafe.stdout" 2>"$tmp_dir/unsafe.stderr"; then
+  echo "unsafe label was accepted" >&2
+  exit 1
+fi
+grep -q -- '--label contains unsafe characters' "$tmp_dir/unsafe.stderr"
+
+echo "test_capture_remote_journal_errors: ok"
diff --git a/scripts/test_collect_remote_samply_artifacts.sh b/scripts/test_collect_remote_samply_artifacts.sh
new file mode 100755
index 000000000..2c2654553
--- /dev/null
+++ b/scripts/test_collect_remote_samply_artifacts.sh
@@ -0,0 +1,80 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+repo_root=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)
+tmp_dir=$(mktemp -d)
+trap 'rm -rf "$tmp_dir"' EXIT
+
+mock_bin="$tmp_dir/bin"
+mkdir -p "$mock_bin"
+
+cat >"$mock_bin/ssh" <<'EOF'
+#!/usr/bin/env bash
+set -euo pipefail
+printf 'ssh:%s\n' "$*" >>"$CALL_LOG"
+echo "profile.json.gz 128 bytes"
+echo "profile.syms.json 64 bytes"
+EOF
+
+cat >"$mock_bin/scp" <<'EOF'
+#!/usr/bin/env bash
+set -euo pipefail
+if read -r consumed; then
+  printf 'scp-consumed-stdin:%s\n' "$consumed" >>"$CALL_LOG"
+  exit 23
+fi
+printf 'scp:%s\n' "$*" >>"$CALL_LOG"
+dest="${@: -1}"
+mkdir -p "$dest"
+touch "$dest/copied-profile.json.gz"
+EOF
+
+chmod +x "$mock_bin/ssh" "$mock_bin/scp"
+
+mapping="$tmp_dir/nodes.txt"
+cat >"$mapping" <<'EOF'
+vm004 /data/rustfs/hotpath/put-1m
+vm005 /data/rustfs/hotpath/get-1m
+EOF
+
+export CALL_LOG="$tmp_dir/calls.log"
+output=$("$repo_root/scripts/collect_remote_samply_artifacts.sh" \
+  --mapping "$mapping" \
+  --out-dir "$tmp_dir/out" \
+  --ssh-bin "$mock_bin/ssh" \
+  --scp-bin "$mock_bin/scp")
+
+[[ "$output" == "collected_nodes=2" ]]
+[[ -f "$tmp_dir/out/vm004/copied-profile.json.gz" ]]
+[[ -f "$tmp_dir/out/vm005/copied-profile.json.gz" ]]
+[[ "$(grep -c '^scp:' "$CALL_LOG")" -eq 2 ]]
+if grep -q '^scp-consumed-stdin:' "$CALL_LOG"; then
+  echo "scp consumed the mapping loop stdin" >&2
+  exit 1
+fi
+
+bad_mapping="$tmp_dir/bad-nodes.txt"
+printf 'vm004 /\n' >"$bad_mapping"
+if "$repo_root/scripts/collect_remote_samply_artifacts.sh" \
+  --mapping "$bad_mapping" \
+  --out-dir "$tmp_dir/bad-out" \
+  --ssh-bin "$mock_bin/ssh" \
+  --scp-bin "$mock_bin/scp" >"$tmp_dir/bad.stdout" 2>"$tmp_dir/bad.stderr"; then
+  echo "unsafe remote directory was accepted" >&2
+  exit 1
+fi
+grep -q 'path contains unsafe characters' "$tmp_dir/bad.stderr"
+
+unsafe_mapping="$tmp_dir/unsafe-nodes.txt"
+printf 'vm004 /data/rustfs/hotpath/put;rm\n' >"$unsafe_mapping"
+if "$repo_root/scripts/collect_remote_samply_artifacts.sh" \
+  --mapping "$unsafe_mapping" \
+  --out-dir "$tmp_dir/unsafe-out" \
+  --ssh-bin "$mock_bin/ssh" \
+  --scp-bin "$mock_bin/scp" >"$tmp_dir/unsafe.stdout" 2>"$tmp_dir/unsafe.stderr"; then
+  echo "unsafe remote path was accepted" >&2
+  exit 1
+fi
+grep -q 'path contains unsafe characters' "$tmp_dir/unsafe.stderr"
+
+echo "test_collect_remote_samply_artifacts: ok"