Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions lib/hook-utils.sh
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,66 @@ hook::repo_root() {
return 1
}

# Repo-relative form of <file> under <repo-root> — the shape the telemetry
# schema requires of `data.file` ("relative to the consuming repo root").
#
# Both sides go through `cygpath -lm` (long name, forward-slash mixed form)
# when it is available, so the prefix strip compares ONE representation: on
# Windows Git Bash `git rev-parse --show-toplevel` answers with a drive-letter
# path while file_path may arrive in POSIX mount form, and the raw strip never
# matches. On Linux/macOS cygpath is absent and both paths are already POSIX,
# so the strip runs directly.
#
# What survives the strip is not trusted to BE relative. A mount/symlink
# mismatch, or a cygpath that answers for one side and not the other, leaves
# the whole absolute path, which embeds the developer's username and breaks the
# schema contract. Such a path degrades to its basename rather than leaking
# (#1133), and the answer is DISTINGUISHABLE: return 1 and
# HOOK_REPO_RELATIVE_DEGRADED=1. Success (return 0) means the path is genuinely
# repo-relative. Telemetry callers that ignore the status still get the safe
# value; a caller that feeds the result to a TOOL must branch on it, because a
# bare basename resolved against the repo root names a different file.
# FILE_REL=$(hook::repo_relative_path "$FILE" "$REPO_ROOT")
#
# Callers under `set -e` must not take the status from a bare assignment: a
# degraded answer returns 1, and `FILE_REL=$(hook::repo_relative_path ...)`
# would abort the shell. Append `|| <flag>=1` (what every tool-feeding caller
# here does) or `|| :` to keep the failure handled.
# shellcheck disable=SC2034 # public contract: callers may read HOOK_REPO_RELATIVE_DEGRADED
hook::repo_relative_path() {
local file="$1" root="$2" rel="$1"
HOOK_REPO_RELATIVE_DEGRADED=0
# An empty root anchors nothing, and the strip must not run against one:
# `${file#""/}` merely shaves the leading slash, handing back a path that is
# still the caller's absolute path but no longer LOOKS absolute to the
# redaction below, so it would leak with a success status. Skipping the strip
# leaves rel as the input, which the redaction then degrades correctly.
if [[ -n "$root" ]]; then
if command -v cygpath >/dev/null 2>&1; then
local file_lm root_lm
file_lm=$(cygpath -lm "$file" 2>/dev/null)
root_lm=$(cygpath -lm "$root" 2>/dev/null)
if [[ -n "$file_lm" && -n "$root_lm" ]]; then
rel="${file_lm#"$root_lm"/}"
fi
else
rel="${file#"$root"/}"
fi
fi
# POSIX-absolute, drive-letter, and UNC are the three spellings an unstripped
# path arrives in. Trim on either separator: a mixed-form path carries both.
case "$rel" in
/* | [A-Za-z]:* | \\\\*)
rel="${rel##*/}"
rel="${rel##*\\}"
HOOK_REPO_RELATIVE_DEGRADED=1
;;
*) ;; # already repo-relative, nothing to redact
esac
printf '%s' "$rel"
((HOOK_REPO_RELATIVE_DEGRADED == 0))
}

# Buffer a complete JSON payload from stdin, tolerating Windows Win32-pipe
# late-EOF stalls via a bounded read on the inherited fd0. Returns the payload
# on success; returns 1 on empty/incomplete stdin (caller skips), or 2 when the
Expand Down
111 changes: 111 additions & 0 deletions lib/hook-utils.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2732,6 +2732,117 @@ RR_NOGIT="$(mktemp -d)"
repo_root_unresolved "$RR_NOGIT"
rm -rf "$RR_NOGIT"

# --- hook::repo_relative_path: strip, redact, and say which happened ---------
# The helper answers on three channels like the two above: stdout, the return
# code, and HOOK_REPO_RELATIVE_DEGRADED. The return code is the one a caller in
# a command substitution can read, so every case asserts all three.
#
# BOTH arms run on EVERY host. Which arm the helper takes is decided by
# `command -v cygpath`, so each case drives it in a child shell whose PATH holds
# either nothing (the POSIX direct-strip arm) or ONLY a stub cygpath (the
# Windows long-name arm). Gating on the real host's cygpath instead would leave
# whichever arm that host lacks untested everywhere, including on the
# windows-2025 `hook-utils-windows` lane, which exists to cover exactly this.
RRP_DIR="$(mktemp -d)"
mkdir -p "$RRP_DIR/nocyg" "$RRP_DIR/cyg"
# Stand-in for Git Bash's `cygpath -lm`: POSIX mount form (/c/x) to mixed
# Windows form (C:/x). Only the conversion the helper depends on is modeled, and
# modeling it is the point — the arm exists because the two sides of the strip
# arrive in different spellings and must be brought to one.
#
# Builtins only, and an absolute shebang taken from $BASH: the stub runs with
# the near-empty PATH below, where `/usr/bin/env bash` could not resolve bash
# and `cut`/`tr` could not resolve at all.
{
printf '#!%s\n' "$BASH"
cat <<'CYGEOF'
p=""
for a in "$@"; do p="$a"; done
_lower="abcdefghijklmnopqrstuvwxyz"
_upper="ABCDEFGHIJKLMNOPQRSTUVWXYZ"
case "$p" in
/[A-Za-z]/* | /[A-Za-z])
d="${p:1:1}"
rest="${p:2}"
case "$d" in
[a-z])
_pre="${_lower%%"$d"*}"
d="${_upper:${#_pre}:1}"
;;
*) ;;
esac
p="$d:$rest"
;;
*) ;;
esac
printf '%s\n' "$p"
CYGEOF
} >"$RRP_DIR/cyg/cygpath"
chmod +x "$RRP_DIR/cyg/cygpath"

# rrp_case <mode> <label> <file> <root> <expected-out> <expected-degraded>
# mode nocyg → no cygpath on PATH, helper takes the direct-strip arm
# mode cyg → stub cygpath on PATH, helper takes the normalization arm
# The child calls the helper twice on purpose: command substitution captures
# stdout but runs in a subshell, so the global has to be read from a plain call.
# Only shell builtins are used inside, because PATH is deliberately near-empty.
rrp_case() {
local mode="$1" label="$2" file="$3" root="$4" want="$5" want_deg="$6" probe
probe=$(
# $BASH, not a bare `bash`: the PATH below is deliberately near-empty, so a
# bare name could not be resolved. shellcheck cannot see that the quoted
# argument is a bash script, so it reads the (correctly) unexpanded $1/$2/$3
# as a mistake; they are the child's own positional parameters.
# shellcheck disable=SC2016
PATH="$RRP_DIR/$mode" "$BASH" -c '
# shellcheck source=hook-utils.sh
source "$1"
_out=$(hook::repo_relative_path "$2" "$3")
_rc=$?
hook::repo_relative_path "$2" "$3" >/dev/null
printf "%s\n%s\n%s\n" "$_rc" "$HOOK_REPO_RELATIVE_DEGRADED" "$_out"
' _ "$HOOK_DIR/hook-utils.sh" "$file" "$root"
)
local rc flag out
{
read -r rc
read -r flag
read -r out
} <<<"$probe"
if [[ "$out" == "$want" ]] && ((rc == want_deg)) && ((flag == want_deg)); then
ok "repo_relative_path[$mode]: $label"
else
fail "repo_relative_path[$mode] $label: out=$out (want $want) rc=$rc flag=$flag (want $want_deg)"
fi
}

# The POSIX arm.
rrp_case nocyg "strips the repo root" /repo/a/b.md /repo a/b.md 0
rrp_case nocyg "root mismatch redacts to basename" /elsewhere/a/b.md /repo b.md 1
rrp_case nocyg "drive-letter path redacts" 'C:/proj/app/a/b.md' /repo b.md 1
# portability-ok: a literal Windows UNC fixture path; the \s and \b are path
# separators plus a filename, not GNU regex escapes.
rrp_case nocyg "UNC path redacts on the backslash" '\\srv\share\b.md' /repo b.md 1
rrp_case nocyg "an already-relative path passes through" a/b.md /repo a/b.md 0
# Only the trailing-slash prefix strips, so the root passed as the file stays
# absolute and redacts — the caller never receives an unmarked absolute path.
rrp_case nocyg "the root as the file redacts" /repo /repo repo 1
# An empty root anchors nothing. Without the non-empty guard the strip would
# shave the leading slash and hand back srv/proj/repo/a.txt with status 0: still
# the caller's absolute path, no longer matching the redaction's /* arm.
rrp_case nocyg "an empty root redacts instead of shaving the slash" /srv/proj/repo/a.txt "" a.txt 1

# The cygpath arm. The first case is the one the arm exists for: file in POSIX
# mount form, root in drive-letter form. The nocyg control directly below shows
# the same inputs degrading without the normalization, so the case cannot pass
# for the wrong reason.
rrp_case cyg "mount-form file under a drive-letter root strips" /c/repo/a/b.md 'C:/repo' a/b.md 0
rrp_case nocyg "...and the same inputs degrade without cygpath (control)" /c/repo/a/b.md 'C:/repo' b.md 1
rrp_case cyg "both sides already mixed form" 'C:/repo/a/b.md' 'C:/repo' a/b.md 0
rrp_case cyg "a root mismatch still redacts through the cygpath arm" /c/elsewhere/b.md 'C:/repo' b.md 1
rrp_case cyg "an empty root redacts through the cygpath arm" /c/repo/a/b.md "" b.md 1
rm -rf "$RRP_DIR"

# --- hook::bash_parse_segments: unquoted # comments to EOL --------------------
bps_last=()
bps_collect() {
Expand Down
2 changes: 1 addition & 1 deletion plugins/actionlint/.claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json",
"name": "actionlint",
"version": "0.8.25",
"version": "0.8.26",
"description": "Lint GitHub Actions workflow files on edit via actionlint, surfacing findings as advisory context.",
"author": {
"name": "Melodic Software",
Expand Down
22 changes: 22 additions & 0 deletions plugins/actionlint/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,28 @@
All notable changes to the `actionlint` plugin are documented here. Format follows
[Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning.

## [0.8.26]

### Fixed

- **A redacted path no longer becomes the lint target.** `FILE_REL` is passed to
`actionlint` as the file to lint, but since the degrade landed in 0.6.0
(#1133) it can hold a bare basename, which resolves against the repo root to a
workflow that is not there. The lint then found nothing and this advisory hook
reported a clean pass. It now branches on the helper's degrade status and
lints the absolute path in that case. A symlinked-root regression case in
`hooks/actionlint-check.test.sh` pins it: the case fails without the branch.

### Changed

- **The repo-relative path block moved into the shared lib.** This hook's own
`FILE_REL` computation, including the absolute-path degrade added in 0.6.0
(#1133), is now `hook::repo_relative_path` in `hooks/hook-utils.sh`. Emitted
`data.file` values are unchanged; eight sibling call sites that had copied the
block without the degrade pick the fix up through it, and the helper
additionally trims a UNC path on its backslash. Copies stay byte-identical via
`scripts/sync-hook-utils.sh`.

## [0.8.25]

### Changed
Expand Down
42 changes: 17 additions & 25 deletions plugins/actionlint/hooks/actionlint-check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -86,30 +86,15 @@ fi
# Resolve repo root early — used to compute the schema-required repo-relative
# path in data.file.
REPO_ROOT="$(hook::repo_root "$(dirname "$FILE")")"
# Repo-relative path: schema requires "relative to the consuming repo root".
# On Windows Git Bash, git rev-parse --show-toplevel returns a drive-letter path
# while FILE may be in POSIX mount form. Normalize both through cygpath -lm
# (long name, forward-slash mixed form) when available so the prefix strip
# compares the same representation. On Linux/macOS, cygpath is absent and both
# paths are already POSIX. Falls back to raw FILE on any normalization error.
FILE_REL="$FILE"
if command -v cygpath >/dev/null 2>&1; then
_file_lm=$(cygpath -lm "$FILE" 2>/dev/null)
_root_lm=$(cygpath -lm "$REPO_ROOT" 2>/dev/null)
if [[ -n "$_file_lm" && -n "$_root_lm" ]]; then
FILE_REL="${_file_lm#"$_root_lm"/}"
fi
else
FILE_REL="${FILE#"$REPO_ROOT"/}"
fi
# The schema's data.file contract is repo-relative. When the prefix strip did
# not match (mount/symlink mismatch, cygpath disagreement), FILE_REL is still
# an absolute path -- degrade to the basename rather than leaking the absolute
# path into telemetry.
case "$FILE_REL" in
[A-Za-z]:* | /* | \\\\*) FILE_REL="$(basename "$FILE")" ;;
*) ;;
esac
# Repo-relative path, serving two consumers: the schema-required data.file, and
# the argument actionlint runs on from the repo root. A path the prefix strip
# could not make relative degrades to its basename, which is right for telemetry
# but names a DIFFERENT file when resolved against the repo root, so the
# invocation below has to know which of the two it holds. Command substitution
# runs the helper in a subshell, so its HOOK_REPO_RELATIVE_DEGRADED global never
# reaches this scope; the return status is the channel that survives.
FILE_REL_DEGRADED=0
FILE_REL="$(hook::repo_relative_path "$FILE" "$REPO_ROOT")" || FILE_REL_DEGRADED=1

# Build the telemetry data object for the current TOOL/FILE_REL. $1 is the
# findings JSON array. jq is authoritative. The fallback is a fixed empty-shape
Expand Down Expand Up @@ -152,7 +137,14 @@ if ! cd "$REPO_ROOT" 2>/dev/null; then
emit_tel "error" '[]'
exit 0
fi
AL_OUTPUT=$(actionlint -shellcheck= -pyflakes= -- "$FILE_REL" 2>&1)
# The lint target is the repo-relative path so diagnostics echo it, but only
# when it IS repo-relative. A degraded FILE_REL is a bare basename redacted for
# telemetry; resolved against the repo root it names a different workflow or
# none, and this advisory hook would drop real findings silently. Fall back to
# the absolute path there.
AL_TARGET="$FILE_REL"
((FILE_REL_DEGRADED == 0)) || AL_TARGET="$FILE"
AL_OUTPUT=$(actionlint -shellcheck= -pyflakes= -- "$AL_TARGET" 2>&1)
AL_STATUS=$?

# actionlint exits 0 (clean) or 1 (problems found); anything else -- 2 invalid
Expand Down
42 changes: 42 additions & 0 deletions plugins/actionlint/hooks/actionlint-check.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,48 @@ else
fail "actionlint-absent latch/PATH diagnostic wrong: $OUT_MIN"
fi

# --- Symlinked repo root: the lint target must stay the edited file ----------
# The hook cd's to the repo root and passes actionlint a repo-relative path so
# diagnostics read cleanly. That path comes from hook::repo_relative_path, which
# REDACTS to a bare basename when the repo-root prefix strip does not match.
# Reaching one repo through a symlink produces exactly that mismatch: file_path
# keeps the symlinked spelling while `git rev-parse --show-toplevel` answers with
# the physical path. Workflow files always sit under .github/workflows/, so the
# redacted basename resolves against the repo root to a file that is not there,
# and without the degrade branch a real violation vanishes from an advisory hook.
if ln -s "$WORK/symlink-real" "$WORK/symlink-link" 2>/dev/null; then
REPO_SL="$WORK/symlink-real"
new_repo "$REPO_SL"
# shellcheck disable=SC2016 # literal workflow YAML fixture: ${{ }} must stay unexpanded
printf 'name: bad\non: push\njobs:\n build:\n runs-on: ubuntu-latest\n steps:\n - run: echo "${{ steps.missing.outputs.x }}"\n' \
>"$REPO_SL/.github/workflows/violation.yml"
OUT_SL=$(run_hook "$WORK/symlink-link/.github/workflows/violation.yml")
CTX_SL=$(printf '%s' "$OUT_SL" | jq -r '.hookSpecificOutput.additionalContext // ""' 2>/dev/null)
if printf '%s' "$CTX_SL" | grep -q 'missing'; then
ok "symlinked root: the real violation still surfaces (lint target not redacted)"
else
fail "symlinked root: violation lost, hook linted a redacted path: $OUT_SL"
fi
if ! printf '%s' "$CTX_SL" | grep -qiE 'could not read|no such file'; then
ok "symlinked root: no unreadable-target error from a basename-only target"
else
fail "symlinked root: actionlint got a nonexistent target: $CTX_SL"
fi
# The redaction itself must still hold on the telemetry side: data.file is
# the basename, never the absolute path that embeds the developer's username.
SL_OUT="$WORK/sl-telemetry.json"
SL_SINK=$(make_sink "cat >\"$SL_OUT\"")
run_hook_env "$WORK/symlink-link/.github/workflows/violation.yml" \
CLAUDE_PLUGIN_OPTION_ACTIONLINT_ENABLED=true HOOK_TELEMETRY_SINK="$SL_SINK" >/dev/null
if wait_for_sink "$SL_OUT" && [[ "$(jq -r '.data.file' "$SL_OUT" 2>/dev/null)" == "violation.yml" ]]; then
ok "symlinked root: telemetry data.file stays redacted to the basename"
else
fail "symlinked root: data.file was $(jq -r '.data.file' "$SL_OUT" 2>/dev/null)"
fi
else
echo "SKIP: symlinks unavailable on this filesystem -- symlinked-root case skipped"
fi

echo
echo "PASS=$PASS FAIL=$FAIL"
[[ $FAIL -eq 0 ]]
60 changes: 60 additions & 0 deletions plugins/actionlint/hooks/hook-utils.sh
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,66 @@ hook::repo_root() {
return 1
}

# Repo-relative form of <file> under <repo-root> — the shape the telemetry
# schema requires of `data.file` ("relative to the consuming repo root").
#
# Both sides go through `cygpath -lm` (long name, forward-slash mixed form)
# when it is available, so the prefix strip compares ONE representation: on
# Windows Git Bash `git rev-parse --show-toplevel` answers with a drive-letter
# path while file_path may arrive in POSIX mount form, and the raw strip never
# matches. On Linux/macOS cygpath is absent and both paths are already POSIX,
# so the strip runs directly.
#
# What survives the strip is not trusted to BE relative. A mount/symlink
# mismatch, or a cygpath that answers for one side and not the other, leaves
# the whole absolute path, which embeds the developer's username and breaks the
# schema contract. Such a path degrades to its basename rather than leaking
# (#1133), and the answer is DISTINGUISHABLE: return 1 and
# HOOK_REPO_RELATIVE_DEGRADED=1. Success (return 0) means the path is genuinely
# repo-relative. Telemetry callers that ignore the status still get the safe
# value; a caller that feeds the result to a TOOL must branch on it, because a
# bare basename resolved against the repo root names a different file.
# FILE_REL=$(hook::repo_relative_path "$FILE" "$REPO_ROOT")
#
# Callers under `set -e` must not take the status from a bare assignment: a
# degraded answer returns 1, and `FILE_REL=$(hook::repo_relative_path ...)`
# would abort the shell. Append `|| <flag>=1` (what every tool-feeding caller
# here does) or `|| :` to keep the failure handled.
# shellcheck disable=SC2034 # public contract: callers may read HOOK_REPO_RELATIVE_DEGRADED
hook::repo_relative_path() {
local file="$1" root="$2" rel="$1"
HOOK_REPO_RELATIVE_DEGRADED=0
# An empty root anchors nothing, and the strip must not run against one:
# `${file#""/}` merely shaves the leading slash, handing back a path that is
# still the caller's absolute path but no longer LOOKS absolute to the
# redaction below, so it would leak with a success status. Skipping the strip
# leaves rel as the input, which the redaction then degrades correctly.
if [[ -n "$root" ]]; then
if command -v cygpath >/dev/null 2>&1; then
local file_lm root_lm
file_lm=$(cygpath -lm "$file" 2>/dev/null)
root_lm=$(cygpath -lm "$root" 2>/dev/null)
if [[ -n "$file_lm" && -n "$root_lm" ]]; then
rel="${file_lm#"$root_lm"/}"
fi
else
rel="${file#"$root"/}"
fi
fi
# POSIX-absolute, drive-letter, and UNC are the three spellings an unstripped
# path arrives in. Trim on either separator: a mixed-form path carries both.
case "$rel" in
/* | [A-Za-z]:* | \\\\*)
rel="${rel##*/}"
rel="${rel##*\\}"
HOOK_REPO_RELATIVE_DEGRADED=1
;;
*) ;; # already repo-relative, nothing to redact
esac
printf '%s' "$rel"
((HOOK_REPO_RELATIVE_DEGRADED == 0))
}

# Buffer a complete JSON payload from stdin, tolerating Windows Win32-pipe
# late-EOF stalls via a bounded read on the inherited fd0. Returns the payload
# on success; returns 1 on empty/incomplete stdin (caller skips), or 2 when the
Expand Down
Loading