fix(guardrails): stop a UNC file path reaching telemetry whole

plugins/guardrails/.claude-plugin/plugin.json

@@ -147,5 +147,5 @@
       "min": 1
     }
   },
-  "version": "0.29.21"
+  "version": "0.29.22"
 }

plugins/guardrails/CHANGELOG.md

@@ -3,6 +3,41 @@
 All notable changes to the `guardrails` plugin are documented here. Format follows
 [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning.
 
+## [0.29.22]
+
+### Fixed
+
+- **A UNC file path no longer reaches telemetry whole.** `secret-pattern-detection.sh`
+  kept its own copy of the repo-relative computation, and that copy's redaction
+  tested only two of the three absolute spellings: POSIX-absolute and
+  drive-lettered, never UNC. This guard deliberately scans on when no project
+  dir is set, and on that path the copy did no separator folding either, so a
+  `file_path` of the `\\server\share\file` shape matched neither redaction arm
+  and the whole share path, server name included, landed in the envelope's
+  `data.file`. It now calls `hook::repo_relative_path`, which carries the UNC
+  arm, and pairs it with `hook::repo_root` so a file with no project dir is
+  still reported relative to its own checkout instead of collapsing to a bare
+  basename.
+- **A trailing slash on the project dir no longer collapses every path.** The
+  helper strips `"$root/"`, so a root already ending in a separator forms the
+  prefix `/repo//` and matches nothing, degrading every in-project file to its
+  basename. The hand-rolled copies trimmed the separator first and the move to
+  the helper dropped that trim; both call sites now trim it back. A trailing
+  slash is a supported spelling of `CLAUDE_PROJECT_DIR`, which this plugin's
+  own scope tests already exercise. `hook::repo_root` never returns one, so the
+  other call sites of the helper were never exposed.
+
+### Changed
+
+- **The last two hand-rolled path redactions collapse into the shared helper.**
+  `hardcoded-path-check.sh` carried the same duplicated block. Its scope guard
+  exits before the computation whenever the project dir is unset, so the leaking
+  shape was never reachable there and its emitted `data.file` is unchanged; the
+  copy is removed so a third divergent one cannot reappear. In both hooks
+  `file_rel` reaches only the telemetry payload, never a tool argument, and both
+  now resolve it inside `emit_tel`, so a run with no telemetry sink wired does
+  not pay for it at all.
+
 ## [0.29.21]
 
 ### Changed

plugins/guardrails/hooks/guardrails-test-helpers.sh

@@ -29,6 +29,13 @@ bad() {
   FAIL=$((FAIL + 1))
 }
 
+# assert_eq <label> <expected> <actual>. Prefer this over assert_contains for a
+# value with a known exact form: containment passes on any string that merely
+# embeds the expected one, which is how a leaked `\\srv\share\secrets.env` once
+# satisfied a "data.file is the basename" check.
+assert_eq() {
+  if [[ "$3" == "$2" ]]; then ok "$1 ($3)"; else bad "$1: expected '$2', got '$3'"; fi
+}
 # assert_exit <label> <expected> <actual>
 assert_exit() {
   if [[ "$3" == "$2" ]]; then ok "$1 (exit $3)"; else bad "$1: expected exit $2, got $3"; fi

plugins/guardrails/hooks/hardcoded-path-check.sh

@@ -189,31 +189,27 @@ if [[ -n "$PROJECT_ROOT" ]]; then
   fi
 fi
 
-# Repo-relative file for telemetry data.file (best-effort prefix strip).
-FILE_REL="$FILE"
-if [[ -n "$PROJECT_ROOT" ]]; then
-  _root="${PROJECT_ROOT//\\//}"
-  _root="${_root%/}"
-  _fwd="${FILE//\\//}"
-  FILE_REL="${_fwd#"$_root"/}"
-fi
-# Redaction: if the path could not be made repo-relative, emit the basename only
-# — never an absolute path (it would embed the developer's username) into telemetry.
-case "$FILE_REL" in
-/* | [A-Za-z]:*)
-  FILE_REL="${FILE_REL##*/}"
-  FILE_REL="${FILE_REL##*\\}"
-  ;;
-*) ;;
-esac
-
 # Emit one telemetry envelope: $1 status, $2 labels JSON array. Gated on the
-# high-res start stamp and the opt-in sink — the unwired path spawns nothing.
+# high-res start stamp and the opt-in sink — the unwired path spawns nothing,
+# which is also why the repo-relative path is resolved in here rather than at
+# the top level: the default unwired run never pays for it.
 emit_tel() {
   [[ -n "$start" ]] || return 0
   hook::telemetry_enabled || return 0
+  # The helper carries the redaction: a path it could not make repo-relative
+  # comes back as the basename, never an absolute path (which would embed the
+  # developer's username) and never a UNC share (which would name an internal
+  # host). PROJECT_ROOT is the anchor the envelope itself carries, and the
+  # scope guard above guarantees it is set, so no fallback root is needed.
+  # The helper strips "$root/", so a root that already ends in a separator
+  # makes the prefix "/repo//" and matches nothing: every in-project file
+  # would collapse to its basename. PROJECT_ROOT comes from the caller-supplied
+  # CLAUDE_PROJECT_DIR, where a trailing slash is a supported spelling, so trim
+  # it here. The copy this replaced did the same.
+  local file_rel
+  file_rel="$(hook::repo_relative_path "$FILE" "${PROJECT_ROOT%/}")"
   local data
-  data=$(jq -n --arg file "$FILE_REL" --argjson violations "$2" \
+  data=$(jq -n --arg file "$file_rel" --argjson violations "$2" \
     '{tool:"'"$TOOL"'",file:$file,violations:$violations}' 2>/dev/null) ||
     data='{"tool":"","file":"","violations":[]}'
   hook::emit_telemetry "hardcoded-path-check" "PreToolUse" "$1" "$start" "$data" "${CLAUDE_PROJECT_DIR:-}"

plugins/guardrails/hooks/hardcoded-path-check.test.sh

@@ -500,6 +500,54 @@ else
   bad "telemetry: no envelope written on block"
 fi
 
+# --- Telemetry path: the hoisted helper, not a hand-rolled prefix strip ------
+# This hook kept its own copy of the repo-relative computation after the helper
+# was hoisted into hook-utils.sh, and the copy's redaction knew only two of the
+# three absolute spellings (it had no UNC arm). The scope guard above makes the
+# leaking shape unreachable HERE, so no black-box case can tell the two apart on
+# a POSIX host; the duplication is what this pins, so a third divergent copy
+# cannot reappear behind a guard that hides its behavior.
+HOOK_SRC=$(cat "$HOOK")
+assert_contains "path helper: uses hook::repo_relative_path" "$HOOK_SRC" 'hook::repo_relative_path'
+assert_absent "path helper: no hand-rolled prefix strip" "$HOOK_SRC" '_fwd#'
+
+# REGRESSION PIN, NOT A FAIL-THEN-PASS CASE. This one passes against the
+# pre-helper hook too, and is not evidence of the UNC fix. It exists because the
+# computation MOVED into emit_tel and this suite asserted nothing about
+# data.file before, so without it a relocation mistake (an out-of-scope
+# PROJECT_ROOT, an empty file_rel) would pass every other case here.
+#
+# The value that computation produces is unchanged for the only shape that
+# reaches it: the scope guard guarantees a project dir that is a git work tree
+# and a file inside it, so data.file stays repo-relative.
+TELP="$(mktemp "$TEST_TMPDIR/tmp.XXXXXXXXXX")"
+SINKP="$(make_sink "cat >\"$TELP\"")"
+mkdir -p "$TEST_TMPDIR/src"
+env HOOK_TELEMETRY_SINK="$SINKP" CLAUDE_PROJECT_DIR="$TEST_TMPDIR" \
+  bash "$HOOK" <<<"$(write_json "$TEST_TMPDIR/src/run.sh" "cd ${LINUX_HOME}")" >/dev/null 2>&1 || true
+if wait_for_sink "$TELP"; then
+  assert_contains "telemetry: data.file is repo-relative" "$(jq -r '.data.file' "$TELP")" "src/run.sh"
+else
+  bad "telemetry: no envelope written for the repo-relative case"
+fi
+
+# --- Trailing-slash project dir (this one DOES discriminate) ----------------
+# The helper strips "$root/", so a PROJECT_ROOT already ending in a separator
+# makes the prefix "/repo//" and matches nothing: an in-project file collapses
+# to its basename. CLAUDE_PROJECT_DIR is caller-supplied and a trailing slash
+# is a supported spelling, and the hand-rolled copy trimmed it, so the trim has
+# to survive the move to the helper.
+TELPS="$(mktemp "$TEST_TMPDIR/tmp.XXXXXXXXXX")"
+SINKPS="$(make_sink "cat >\"$TELPS\"")"
+env HOOK_TELEMETRY_SINK="$SINKPS" CLAUDE_PROJECT_DIR="$TEST_TMPDIR/" \
+  bash "$HOOK" <<<"$(write_json "$TEST_TMPDIR/src/run.sh" "cd ${LINUX_HOME}")" >/dev/null 2>&1 || true
+if wait_for_sink "$TELPS"; then
+  assert_eq "trailing-slash project dir: data.file stays repo-relative" \
+    "src/run.sh" "$(jq -r '.data.file' "$TELPS")"
+else
+  bad "trailing-slash project dir: no envelope written"
+fi
+
 # ===================== PAYLOAD-SIZE BOUNDARY (regression) ====================
 # Guards the here-string deadlock. Bash delivers `<<<` through a pipe it fills
 # ITSELF before the reader is exec'd, and it appends a newline — so a payload of

plugins/guardrails/hooks/secret-pattern-detection.sh

@@ -147,31 +147,32 @@ NotebookEdit) CONTENT="${HOOK_JQ_FIELDS[4]}" ;;
 esac
 [[ -n "${CONTENT:-}" ]] || exit 0
 
-# Repo-relative file for telemetry data.file (best-effort prefix strip).
-FILE_REL="$FILE"
-if [[ -n "${CLAUDE_PROJECT_DIR:-}" ]]; then
-  _root="${CLAUDE_PROJECT_DIR//\\//}"
-  _root="${_root%/}"
-  _fwd="${FILE//\\//}"
-  FILE_REL="${_fwd#"$_root"/}"
-fi
-# Redaction: if the path could not be made repo-relative, emit the basename only
-# — never an absolute path (it would embed the developer's username) into telemetry.
-case "$FILE_REL" in
-/* | [A-Za-z]:*)
-  FILE_REL="${FILE_REL##*/}"
-  FILE_REL="${FILE_REL##*\\}"
-  ;;
-*) ;;
-esac
-
 # Emit one telemetry envelope: $1 status, $2 labels JSON array. Gated on the
-# high-res start stamp and the opt-in sink — the unwired path spawns nothing.
+# high-res start stamp and the opt-in sink — the unwired path spawns nothing,
+# which is also why the repo-relative path is resolved in here rather than at
+# the top level: the default unwired run never pays for it.
 emit_tel() {
   [[ -n "$start" ]] || return 0
   hook::telemetry_enabled || return 0
+  # The helper carries the redaction: a path it could not make repo-relative
+  # comes back as the basename, never an absolute path (which would embed the
+  # developer's username) and never a UNC share (which would name an internal
+  # host). CLAUDE_PROJECT_DIR is the anchor the envelope itself carries, so
+  # data.file is expressed against that same root when it is set. This hook
+  # deliberately scans on WITHOUT one (the scope guard above falls through
+  # rather than skipping), and an unanchored path can only degrade to a bare
+  # basename, so resolve the file's own checkout for that case.
+  local file_rel root="${CLAUDE_PROJECT_DIR:-}"
+  [[ -n "$root" ]] || root="$(hook::repo_root "$(dirname "$FILE")")"
+  # The helper strips "$root/", so a root that already ends in a separator
+  # makes the prefix "/repo//" and matches nothing: every in-project file
+  # would collapse to its basename. CLAUDE_PROJECT_DIR is caller-supplied and
+  # a trailing slash is a supported spelling, so trim it here. The copy this
+  # replaced did the same, and hook::repo_root never returns one.
+  root="${root%/}"
+  file_rel="$(hook::repo_relative_path "$FILE" "$root")"
   local data
-  data=$(jq -n --arg file "$FILE_REL" --argjson violations "$2" \
+  data=$(jq -n --arg file "$file_rel" --argjson violations "$2" \
     '{tool:"'"$TOOL"'",file:$file,violations:$violations}' 2>/dev/null) ||
     data='{"tool":"","file":"","violations":[]}'
   hook::emit_telemetry "secret-pattern-detection" "PreToolUse" "$1" "$start" "$data" "${CLAUDE_PROJECT_DIR:-}"

plugins/guardrails/hooks/secret-pattern-detection.test.sh

@@ -187,6 +187,93 @@ else
   bad "redaction: no envelope written"
 fi
 
+# --- Telemetry path: the hoisted helper, not a hand-rolled prefix strip ------
+# This hook kept its own copy of the repo-relative computation after the helper
+# was hoisted into hook-utils.sh, and the copy's redaction knew only two of the
+# three absolute spellings. Pin the helper so a third copy cannot reappear.
+assert_contains "path helper: uses hook::repo_relative_path" "$HOOK_SRC" 'hook::repo_relative_path'
+assert_absent "path helper: no hand-rolled prefix strip" "$HOOK_SRC" '_fwd#'
+
+# Telemetry path helper fixtures. A file_path is read as a string, but the
+# no-project-dir cases resolve a root from the file's own checkout, so these
+# need to exist on disk. Anchor on the toplevel git reports rather than on
+# mktemp's answer: on macOS mktemp hands back /var/... where git reports
+# /private/var/..., and the prefix strip would fail for the wrong reason.
+PATHREPO="$TEST_TMPDIR/pathrepo"
+mkdir -p "$PATHREPO/src"
+git -C "$PATHREPO" init -q
+PATHREPO_TL="$(git -C "$PATHREPO" rev-parse --show-toplevel)"
+
+# telemetry_file <file_path> -> data.file from the envelope this hook emits.
+# CLAUDE_PROJECT_DIR stays unset (the file scope guard above falls through
+# rather than skipping when there is no project, so the hook still scans).
+telemetry_file() {
+  local tel sink
+  tel="$(mktemp "$TEST_TMPDIR/tmp.XXXXXXXXXX")"
+  sink="$(make_sink "cat >\"$tel\"")"
+  env HOOK_TELEMETRY_SINK="$sink" bash "$HOOK" \
+    <<<"$(write_json "$1" "config = '$AWS_TOKEN'")" >/dev/null 2>&1 || true
+  if wait_for_sink "$tel"; then jq -r '.data.file' "$tel"; else printf '<no-envelope>'; fi
+}
+
+# --- UNC file_path with no project dir: the leak --------------------------
+# A Windows UNC path is neither POSIX-absolute nor drive-lettered, so a
+# redaction that tests only those two spellings passes the WHOLE share path
+# through — server name and all — into the envelope. The share host is exactly
+# the kind of internal name telemetry must not carry.
+UNC_HOST='srv'
+# shellcheck disable=SC1003  # BS is a literal single backslash, not a quote escape
+BS='\'
+UNC_FILE="${BS}${BS}${UNC_HOST}${BS}share${BS}secrets.env"
+# Equality, not containment: the leaked path ENDS in the basename, so a
+# containment check passes against the pre-fix hook for the wrong reason.
+df=$(telemetry_file "$UNC_FILE")
+assert_eq "UNC/no-project: data.file is exactly the basename" "secrets.env" "$df"
+assert_absent "UNC/no-project: data.file keeps no backslash" "$df" "$BS"
+assert_absent "UNC/no-project: data.file drops the share host" "$df" "$UNC_HOST"
+
+# --- Ordinary in-repo file with no project dir ------------------------------
+# With no project dir the hand-rolled copy resolved no root at all, so every
+# in-repo path degraded to a bare basename and the envelope lost the location
+# the schema asks for. The helper is paired with hook::repo_root, which answers
+# from the file's own checkout.
+df=$(telemetry_file "$PATHREPO_TL/src/config.env")
+assert_contains "in-repo/no-project: data.file is repo-relative" "$df" "src/config.env"
+assert_absent "in-repo/no-project: data.file is not absolute" "$df" "$PATHREPO_TL"
+
+# --- Symlinked checkout ------------------------------------------------------
+# A real repo plus a symlink to it. Reached through the symlink, `git rev-parse
+# --show-toplevel` answers with the PHYSICAL path, so a file_path arriving in
+# the symlink spelling cannot be prefix-stripped by the root the helper is
+# handed. Both spellings are pinned: the physical one must still come back
+# repo-relative, and the symlink one must degrade to a basename rather than
+# leak the resolved physical path the fallback just computed.
+LINKREPO="$TEST_TMPDIR/linkrepo"
+ln -s "$PATHREPO_TL" "$LINKREPO"
+df=$(telemetry_file "$PATHREPO_TL/src/config.env")
+assert_contains "symlinked repo, physical spelling: repo-relative" "$df" "src/config.env"
+df=$(telemetry_file "$LINKREPO/src/config.env")
+assert_contains "symlinked repo, symlink spelling: basename" "$df" "config.env"
+assert_absent "symlinked repo, symlink spelling: no path separator" "$df" "/"
+
+# --- Trailing-slash project dir ---------------------------------------------
+# The helper strips "$root/", so a root already ending in a separator makes the
+# prefix "/repo//" and matches nothing: every in-project file would collapse to
+# its basename and the envelope would lose the location. A trailing slash is a
+# supported spelling of CLAUDE_PROJECT_DIR (the scope test above uses one), and
+# the hand-rolled copy this replaced trimmed it, so the trim has to survive the
+# move to the helper.
+TELTS="$(mktemp "$TEST_TMPDIR/tmp.XXXXXXXXXX")"
+SINKTS="$(make_sink "cat >\"$TELTS\"")"
+env HOOK_TELEMETRY_SINK="$SINKTS" CLAUDE_PROJECT_DIR="$PATHREPO_TL/" bash "$HOOK" \
+  <<<"$(write_json "$PATHREPO_TL/src/config.env" "config = '$AWS_TOKEN'")" >/dev/null 2>&1 || true
+if wait_for_sink "$TELTS"; then
+  assert_eq "trailing-slash project dir: data.file stays repo-relative" \
+    "src/config.env" "$(jq -r '.data.file' "$TELTS")"
+else
+  bad "trailing-slash project dir: no envelope written"
+fi
+
 # ===================== PAYLOAD-SIZE BOUNDARY (regression) ====================
 # Guards the here-string deadlock. Bash delivers `<<<` through a pipe it fills
 # ITSELF before the reader is exec'd, and it appends a newline — so a payload of