diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ca629ac82..2b6b542e4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -202,20 +202,10 @@ jobs: timeout-minutes: 15 steps: - name: Aggregate lane results + # Derived from the needs graph above — the single source of truth for the + # lane list. Adding a lane to needs automatically extends this check. env: - RESULTS: >- - ${{ needs.markdown.result }} - ${{ needs.typos.result }} - ${{ needs.gitleaks.result }} - ${{ needs.editorconfig.result }} - ${{ needs.shellcheck.result }} - ${{ needs.actionlint.result }} - ${{ needs.jsonschema.result }} - ${{ needs.exec-bit.result }} - ${{ needs.machine-specific-paths.result }} - ${{ needs.eol-renormalize.result }} - ${{ needs.comment-hygiene.result }} - ${{ needs.zizmor.result }} + RESULTS: ${{ join(needs.*.result, ' ') }} run: | for r in $RESULTS; do case "$r" in diff --git a/plugins/markdown-formatter/hooks/hook-utils.sh b/plugins/markdown-formatter/hooks/hook-utils.sh index efe874946..6342c5026 100644 --- a/plugins/markdown-formatter/hooks/hook-utils.sh +++ b/plugins/markdown-formatter/hooks/hook-utils.sh @@ -18,16 +18,29 @@ hook::check_enabled() { fi } -# Normalize a path: backslashes → forward slashes, then POSIX/lowercase drive -# letter → uppercase Windows drive letter (idempotent). On macOS/Linux the -# regex does not match (multi-char first segments) — no-op. +# Normalize a path for the membership comparison below: backslashes → forward +# slashes, and — only on Windows/MSYS, whose filesystem is case-insensitive — +# fold a leading drive (POSIX `/c/...` or `c:/...`) to an upper-case drive +# letter + lower-cased remainder so the byte-exact comparison is effectively +# case-insensitive. The fold is gated on the host (OSTYPE), NOT on the path +# shape: on a case-sensitive POSIX filesystem a real single-letter top-level +# directory such as `/c/Repo` must pass through unchanged, otherwise it would +# collapse with `/c/repo` and the membership guard would admit a sibling +# outside CLAUDE_PROJECT_DIR. The result is used ONLY for comparison; the +# emitted path is always the caller's original. hook::normalize_path() { local p="${1//\\//}" - if [[ "$p" =~ ^/([a-zA-Z])/ || "$p" =~ ^([a-zA-Z]):/ ]]; then - printf '%s' "${BASH_REMATCH[1]^}:${p:2}" - else - printf '%s' "$p" - fi + case "${OSTYPE:-}" in + msys* | cygwin* | win32) + if [[ "$p" =~ ^/([a-zA-Z])/ || "$p" =~ ^([a-zA-Z]):/ ]]; then + local rest="${p:2}" + printf '%s' "${BASH_REMATCH[1]^}:${rest,,}" + return + fi + ;; + *) ;; # POSIX hosts: case-sensitive FS, no drive fold — pass through below + esac + printf '%s' "$p" } # Parse file_path from PostToolUse JSON on stdin; validate existence and (when @@ -43,7 +56,11 @@ hook::read_file_path() { local norm_file norm_project norm_file=$(hook::normalize_path "$file") norm_project=$(hook::normalize_path "${CLAUDE_PROJECT_DIR}") - if [[ "$norm_file" != "$norm_project"* ]]; then + norm_project="${norm_project%/}" + # Anchor on a path-segment boundary: accept the project root itself or a + # child under it, but not a sibling whose name merely shares the prefix + # (e.g. /c/repo must not admit /c/repo-backup/...). + if [[ "$norm_file" != "$norm_project" && "$norm_file" != "$norm_project"/* ]]; then return 1 fi fi diff --git a/plugins/markdown-formatter/hooks/hook-utils.test.sh b/plugins/markdown-formatter/hooks/hook-utils.test.sh index aa33a56d7..36b01e75b 100755 --- a/plugins/markdown-formatter/hooks/hook-utils.test.sh +++ b/plugins/markdown-formatter/hooks/hook-utils.test.sh @@ -39,6 +39,20 @@ EOF printf '%s' "$s" } +# wait_for_sink [max_polls] → block until is non-empty (the +# fire-and-forget sink has flushed) or the bound elapses. Polls in 20ms steps so +# the assertion fires as soon as the write lands instead of racing a fixed sleep +# (sink dispatch is a freshly-spawned process; spawn latency varies, especially +# on Windows Git Bash). Returns non-zero on timeout so negative cases can assert. +wait_for_sink() { + local f="$1" tries="${2:-150}" + while (( tries-- > 0 )); do + [[ -s "$f" ]] && return 0 + sleep 0.02 + done + return 1 +} + # --- Test 1: HOOK_TELEMETRY_SINK unset → returns 0, no output ---------------- unset HOOK_TELEMETRY_SINK 2>/dev/null || true out=$(hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$EPOCHREALTIME" '{"tool":"Write","file":"foo.md","findings":[]}' 2>/dev/null) @@ -54,22 +68,20 @@ else fail "sink unset: unexpected output: $out" fi -# --- Test 2: jq absent → emit skipped, no error, returns 0 ------------------ -# Shadow jq with a function that returns 127 to simulate absence. -# The HOOK_TELEMETRY_SINK env var and the jq shadow are scoped to the subshell -# so they never pollute the outer test context. +# --- Test 2: jq absent → fail-open (returns 0, no output) ------------------- +# Make `command -v jq` genuinely fail by running with a PATH that contains no +# jq. A shell-function shadow does NOT exercise the guard: `command -v jq` +# reports a defined function as present, so the absence branch is never taken. +# emit_telemetry uses only shell builtins until its jq calls, so an empty PATH +# is sufficient. Scoped to the command so PATH/sink never leak into the suite. +EMPTY_BIN="$(mktemp -d)" # shellcheck disable=SC2030,SC2031 out_nojq=$( export HOOK_TELEMETRY_SINK="cat" - ( - unset _MDFMT_HOOK_UTILS_LOADED 2>/dev/null || true - jq() { return 127; } - export -f jq - source "$HOOK_DIR/hook-utils.sh" - hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$EPOCHREALTIME" '{"tool":"Write","file":"foo.md","findings":[]}' 2>/dev/null - ) + PATH="$EMPTY_BIN" hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$EPOCHREALTIME" '{"tool":"Write","file":"foo.md","findings":[]}' 2>/dev/null ) rc_nojq=$? +rmdir "$EMPTY_BIN" 2>/dev/null || true if [[ $rc_nojq -eq 0 ]]; then ok "jq absent: returns 0" else @@ -93,8 +105,7 @@ export HOOK_TELEMETRY_SINK data_json='{"tool":"Write","file":"docs/foo.md","findings":["docs/foo.md:12 MD013/line-length"]}' start=$EPOCHREALTIME hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$start" "$data_json" 2>/dev/null -# Allow the background process to finish -sleep 0.2 +wait_for_sink "$SINK_FILE" unset HOOK_TELEMETRY_SINK if [[ -s "$SINK_FILE" ]]; then @@ -165,10 +176,12 @@ if [[ -s "$SINK_FILE" ]]; then # We verify by checking the TZ=UTC prefix actually produces UTC utc_h=$(TZ=UTC date +%H) ts_h="${ts:11:2}" - # Allow ±1h boundary tolerance - h_diff=$(( ${ts_h#0} - ${utc_h#0} )) - h_diff=${h_diff#-} - if [[ $h_diff -le 1 ]]; then + # Circular ±1h tolerance to absorb an hour-boundary straddle between the emit + # and this check — including the 23→00 midnight wrap, where a linear distance + # would read 23 and false-fail once per day. (#0 strips the leading zero so + # 00–09 are not misread as octal inside (( )).) + h_diff=$(( ( ${ts_h#0} - ${utc_h#0} + 24 ) % 24 )) + if [[ $h_diff -le 1 || $h_diff -ge 23 ]]; then ok "timestamp: UTC hour aligns with system UTC" else fail "timestamp: hour drift vs UTC: ts_h=$ts_h utc_h=$utc_h diff=$h_diff" @@ -198,7 +211,7 @@ SINK_FILE2="$(mktemp)" HOOK_TELEMETRY_SINK="$(make_sink "$SINK_FILE2")" export HOOK_TELEMETRY_SINK hook::emit_telemetry "markdown-format" "PostToolUse" "skipped" "$EPOCHREALTIME" '{"tool":"","file":"","findings":[]}' 2>/dev/null -sleep 0.2 +wait_for_sink "$SINK_FILE2" unset HOOK_TELEMETRY_SINK if [[ -s "$SINK_FILE2" ]]; then @@ -227,7 +240,7 @@ EOF chmod +x "$ROOT7/$REL7" export HOOK_TELEMETRY_SINK="$REL7" hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$EPOCHREALTIME" '{"tool":"Write","file":"x.md","findings":[]}' "$ROOT7" 2>/dev/null -sleep 0.2 +wait_for_sink "$OUT7" unset HOOK_TELEMETRY_SINK if [[ -s "$OUT7" ]] && [[ "$(jq -r '.hook' "$OUT7")" == "markdown-format" ]]; then ok "relative sink: resolved against repo_root arg, envelope delivered" @@ -247,7 +260,7 @@ EOF chmod +x "$ROOT8/.claude/hooks/sink.sh" export HOOK_TELEMETRY_SINK=".claude/hooks/sink.sh" CLAUDE_PROJECT_DIR="$ROOT8" hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$EPOCHREALTIME" '{"tool":"Write","file":"x.md","findings":[]}' 2>/dev/null -sleep 0.2 +wait_for_sink "$OUT8" unset HOOK_TELEMETRY_SINK if [[ -s "$OUT8" ]]; then ok "relative sink: resolved against CLAUDE_PROJECT_DIR fallback" @@ -279,7 +292,7 @@ OUT10="$(mktemp)" ABS10="$(make_sink "$OUT10")" # mktemp path is absolute export HOOK_TELEMETRY_SINK="$ABS10" hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$EPOCHREALTIME" '{"tool":"Write","file":"x.md","findings":[]}' "/some/ignored/root" 2>/dev/null -sleep 0.2 +wait_for_sink "$OUT10" unset HOOK_TELEMETRY_SINK if [[ -s "$OUT10" ]] && [[ "$(jq -r '.hook' "$OUT10")" == "markdown-format" ]]; then ok "absolute sink: passes through unchanged (repo_root ignored)" @@ -288,6 +301,43 @@ else fi rm -f "$ABS10" "$OUT10" +# --- Test 11: hook::normalize_path is host-gated on OSTYPE -------------------- +# The drive-letter fold is for Windows/MSYS only (case-insensitive FS). On a +# case-sensitive POSIX host a real single-letter top dir like /c/Repo must pass +# through unchanged, or the membership guard collapses it with /c/repo and +# admits a sibling outside CLAUDE_PROJECT_DIR. normalize_path reads the shell's +# OSTYPE, so override it in a subshell per case (the global stays intact). +norm_as() { ( OSTYPE="$1"; hook::normalize_path "$2" ); } +assert_norm() { # + local got + got="$(norm_as "$1" "$2")" + if [[ "$got" == "$3" ]]; then + ok "normalize_path[$1]: $4" + else + fail "normalize_path[$1]: $4 — expected '$3', got '$got'" + fi +} + +# Windows/MSYS: fold both the POSIX /c/ and colon c:/ drive forms. +assert_norm msys "/c/Repo" "C:/repo" "msys folds /c/Repo → C:/repo" +assert_norm msys "C:/Repo" "C:/repo" "msys folds C:/Repo → C:/repo" +assert_norm msys "/c/Repo/Sub" "C:/repo/sub" "msys folds nested path" +assert_norm msys 'C:\Repo\x' "C:/repo/x" "msys: backslashes → slashes then fold" +assert_norm cygwin "/c/Repo" "C:/repo" "cygwin folds like msys" + +# POSIX: NO fold — single-letter top dirs stay distinct (the regression guard). +assert_norm linux-gnu "/c/Repo" "/c/Repo" "linux leaves /c/Repo unchanged" +assert_norm linux-gnu "/c/repo" "/c/repo" "linux leaves /c/repo unchanged" +assert_norm linux-gnu "/opt/App/Sub" "/opt/App/Sub" "linux leaves normal path unchanged" +assert_norm linux-gnu 'a\b' "a/b" "linux still converts backslashes" + +# Explicit guard: on POSIX the two casings must NOT collapse to one value. +if [[ "$(norm_as linux-gnu /c/Repo)" != "$(norm_as linux-gnu /c/repo)" ]]; then + ok "normalize_path[linux-gnu]: /c/Repo and /c/repo stay distinct" +else + fail "normalize_path[linux-gnu]: /c/Repo and /c/repo collapsed (membership-guard regression)" +fi + echo echo "PASS=$PASS FAIL=$FAIL" [[ $FAIL -eq 0 ]] diff --git a/plugins/markdown-formatter/hooks/markdown-format.sh b/plugins/markdown-formatter/hooks/markdown-format.sh index a31e1f176..355b2fe21 100755 --- a/plugins/markdown-formatter/hooks/markdown-format.sh +++ b/plugins/markdown-formatter/hooks/markdown-format.sh @@ -31,7 +31,6 @@ case "$FILE" in esac TOOL=$(printf '%s' "$INPUT" | jq -r '.tool_name // empty' 2>/dev/null) -TOOL="${TOOL:-}" # Resolve repo root early — needed for CWD-anchored config discovery and for # computing the schema-required repo-relative path in data.file. @@ -53,6 +52,19 @@ else FILE_REL="${FILE#"$REPO_ROOT"/}" fi +# Build the telemetry data object for the current TOOL/FILE_REL. $1 is the +# findings JSON array. jq is authoritative; the string fallback keeps telemetry +# best-effort if jq fails (findings collapse to [] since they can't be re-quoted +# safely without jq). +build_data_json() { + jq -n \ + --arg tool "$TOOL" \ + --arg file "$FILE_REL" \ + --argjson findings "$1" \ + '{tool:$tool,file:$file,findings:$findings}' 2>/dev/null \ + || printf '{"tool":"%s","file":"%s","findings":[]}' "$TOOL" "$FILE_REL" +} + MDLINT=() if command -v markdownlint-cli2 >/dev/null 2>&1; then MDLINT=(markdownlint-cli2) @@ -60,22 +72,14 @@ elif command -v npx >/dev/null 2>&1; then MDLINT=(npx markdownlint-cli2) else # markdownlint unavailable — emit skipped telemetry then exit cleanly. - data_json=$(jq -n \ - --arg tool "$TOOL" \ - --arg file "$FILE_REL" \ - --argjson findings '[]' \ - '{tool:$tool,file:$file,findings:$findings}' 2>/dev/null) || data_json="{\"tool\":\"$TOOL\",\"file\":\"$FILE_REL\",\"findings\":[]}" + data_json=$(build_data_json '[]') hook::emit_telemetry "markdown-format" "PostToolUse" "skipped" "$start" "$data_json" "$REPO_ROOT" exit 0 fi if FIX_OUTPUT=$(cd "$REPO_ROOT" && "${MDLINT[@]}" --fix "$FILE" 2>&1); then # Clean after fix — emit ok with empty findings. - data_json=$(jq -n \ - --arg tool "$TOOL" \ - --arg file "$FILE_REL" \ - --argjson findings '[]' \ - '{tool:$tool,file:$file,findings:$findings}' 2>/dev/null) || data_json="{\"tool\":\"$TOOL\",\"file\":\"$FILE_REL\",\"findings\":[]}" + data_json=$(build_data_json '[]') hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$start" "$data_json" "$REPO_ROOT" exit 0 fi @@ -88,19 +92,21 @@ fi # Violation lines are identified by the " MD/" rule-code pattern. hook::ctx_reset hook::ctx_append "markdown-format: $(basename "$FILE") has markdownlint findings:" -FINDINGS_JSON='[]' +findings_raw="" while IFS= read -r line; do hook::ctx_append " $line" if [[ "$line" =~ [[:space:]]MD[0-9]+/ ]]; then - FINDINGS_JSON=$(printf '%s' "$FINDINGS_JSON" | jq --arg l "$line" '. + [$l]' 2>/dev/null) || true + findings_raw+="$line"$'\n' fi done <<<"$FIX_OUTPUT" hook::ctx_flush PostToolUse -data_json=$(jq -n \ - --arg tool "$TOOL" \ - --arg file "$FILE_REL" \ - --argjson findings "$FINDINGS_JSON" \ - '{tool:$tool,file:$file,findings:$findings}' 2>/dev/null) || data_json="{\"tool\":\"$TOOL\",\"file\":\"$FILE_REL\",\"findings\":[]}" +# Build the findings array in one jq pass (one JSON string per matched line). +FINDINGS_JSON='[]' +if [[ -n "$findings_raw" ]]; then + FINDINGS_JSON=$(printf '%s' "$findings_raw" | jq -R . | jq -s . 2>/dev/null) || FINDINGS_JSON='[]' +fi + +data_json=$(build_data_json "$FINDINGS_JSON") hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$start" "$data_json" "$REPO_ROOT" exit 0 diff --git a/plugins/markdown-formatter/hooks/markdown-format.test.sh b/plugins/markdown-formatter/hooks/markdown-format.test.sh index 0f33ee2ce..b4c3fa3fa 100755 --- a/plugins/markdown-formatter/hooks/markdown-format.test.sh +++ b/plugins/markdown-formatter/hooks/markdown-format.test.sh @@ -49,6 +49,19 @@ make_sink() { printf '%s' "$s" } +# wait_for_sink [max_polls] → block until is non-empty (the +# fire-and-forget sink has flushed) or the bound elapses, polling in 20ms steps. +# Replaces a fixed sleep so delivery assertions fire as soon as the write lands +# instead of racing variable process-spawn latency (notably on Windows Git Bash). +wait_for_sink() { + local f="$1" tries="${2:-150}" + while (( tries-- > 0 )); do + [[ -s "$f" ]] && return 0 + sleep 0.02 + done + return 1 +} + # --- Build the throwaway consumer repo -------------------------------------- REPO="$WORK/consumer" mkdir -p "$REPO" @@ -214,7 +227,7 @@ printf '# Doc T\n\n## Section\n\ntext\n\n## Section\n\nmore text\n' >"$REPO/fixt _OUT_T="$(cd "$UNRELATED" && printf '{"tool_input":{"file_path":"%s"},"tool_name":"Write"}' "$REPO/fixtureT.md" \ | env -u CLAUDE_PROJECT_DIR HOOK_MARKDOWN_FORMAT_ENABLED=true HOOK_TELEMETRY_SINK="$STUB_SINK" bash "$HOOK")" RC_T=$? -sleep 0.3 # allow background sink to flush +wait_for_sink "$TEL_FILE" if [[ $RC_T -eq 0 ]]; then ok "telemetry/stub-sink: hook exit 0"; else fail "telemetry/stub-sink: hook exit $RC_T"; fi @@ -288,7 +301,7 @@ printf '# Clean Doc\n\nSome text.\n' >"$REPO/fixtureClean.md" _OUT_CLEAN="$(cd "$UNRELATED" && printf '{"tool_input":{"file_path":"%s"},"tool_name":"Write"}' "$REPO/fixtureClean.md" \ | env -u CLAUDE_PROJECT_DIR HOOK_MARKDOWN_FORMAT_ENABLED=true HOOK_TELEMETRY_SINK="$STUB_CLEAN" bash "$HOOK")" RC_CLEAN=$? -sleep 0.3 +wait_for_sink "$TEL_CLEAN" if [[ $RC_CLEAN -eq 0 ]]; then ok "telemetry/clean: hook exit 0"; else fail "telemetry/clean: hook exit $RC_CLEAN"; fi if [[ -s "$TEL_CLEAN" ]]; then @@ -312,7 +325,7 @@ printf '# Failing Sink Doc\n\nSome text.\n' >"$REPO/fixtureFailSink.md" _OUT_FS="$(cd "$UNRELATED" && printf '{"tool_input":{"file_path":"%s"},"tool_name":"Write"}' "$REPO/fixtureFailSink.md" \ | env -u CLAUDE_PROJECT_DIR HOOK_MARKDOWN_FORMAT_ENABLED=true HOOK_TELEMETRY_SINK="$FAIL_SINK" bash "$HOOK")" RC_FS=$? -sleep 0.3 +wait_for_sink "$FAIL_SINK_FILE" if [[ $RC_FS -eq 0 ]]; then ok "telemetry/fail-sink: hook exit 0 despite sink failure"; else fail "telemetry/fail-sink: hook exit $RC_FS, expected 0"; fi rm -f "$FAIL_SINK_FILE" @@ -348,7 +361,7 @@ LEAK_SINK="$(make_sink "cat >\"$TEL_LEAK\"")" printf '# Leak Doc\n\n## Section\n\ntext\n\n## Section\n\nmore text\n' >"$REPO/fixtureLeakCheck.md" OUT_LEAK="$(cd "$UNRELATED" && printf '{"tool_input":{"file_path":"%s"},"tool_name":"Write"}' "$REPO/fixtureLeakCheck.md" \ | env -u CLAUDE_PROJECT_DIR HOOK_MARKDOWN_FORMAT_ENABLED=true HOOK_TELEMETRY_SINK="$LEAK_SINK" bash "$HOOK")" -sleep 0.3 +wait_for_sink "$TEL_LEAK" # The hook stdout must NOT contain the telemetry envelope's top-level keys if printf '%s' "$OUT_LEAK" | jq -e '.schema_version' >/dev/null 2>&1; then