You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Migrates the EOL-normalization PostToolUse hook into a repo-agnostic plugin. Bundles normalize-eol.sh via ${CLAUDE_PLUGIN_ROOT}; consumer conventions from .gitattributes/.editorconfig; kill switch HOOK_EOL_NORMALIZER_ENABLED; opt-in telemetry. Security-hardened (-- arg terminators + mktemp staging). 27 contract tests; plugin validate clean. Clean-tier wave Phase 1.
Note
Medium Risk
The hook mutates files on disk immediately after agent edits; guards and best-effort behavior limit damage, but wrong attribute resolution could still rewrite content outside intent.
Overview
Introduces a new eol-normalizer formatting plugin and lists it in the marketplace catalog.
On PostToolUse for Write|Edit, the hook reads the edited path from stdin (with Windows Git Bash–safe single read), resolves the consuming repo root, and runs normalize_eol_file so working-tree endings match .gitattributes (eol=lf → CRLF→LF, eol=crlf → LF→CRLF on all platforms). Paths without eol=, -text, or text=auto binaries (NUL sniff) are left alone. The hook is advisory (always exit 0), disable via HOOK_EOL_NORMALIZER_ENABLED, and optionally emits telemetry through the shared hook-utils copy bundled in the plugin.
The plugin ships normalize-eol.sh (perl with tr/awk + mktemp fallbacks that preserve file mode), hooks.json wiring, README, and a black-box eol-normalizer.test.sh suite covering LF/CRLF arms, kill switch, binary/-text guards, and telemetry envelopes.
Reviewed by Cursor Bugbot for commit dfb3e29. Bugbot is set up for automated code reviews on this repo. Configure here.
Symmetric working-tree line-ending normalization on Write|Edit, driven by
the consuming repo's own .gitattributes via git check-attr. LF arm runs on
every OS; CRLF arm self-gates to Windows. Advisory (exit 0), kill switch
HOOK_EOL_NORMALIZER_ENABLED, opt-in telemetry envelope to HOOK_TELEMETRY_SINK.
Bundles normalize-eol.sh in-plugin; hook-utils.sh is a byte-identical copy of
the marketplace lib. Ports the medley hook's black-box contract tests as a
self-contained, cwd-independent suite.
Argument-injection hardening (CWE-88): add `--` option terminators before
the filename in the perl, mv, and rm invocations so a path beginning with `-`
cannot be parsed as an option.
Predictable temp file (CWE-377): replace fixed `.lf.tmp`/`.crlf.tmp` staging
in the non-perl tr/awk fallback arms with a same-dir `mktemp "${file}.XXXXXX"`,
preserving the atomic same-filesystem mv.
EOL normalization semantics unchanged (contract test suite 27/27 green).
Intentionally diverges from the medley source copy (retained for lefthook use);
the medley-side fix is queued for the Phase 4 cutover boy-scout pass.
Well-constructed plugin. The architecture is sound: .gitattributes-driven policy, file-anchored repo-root resolution, idempotent transforms, and an always-exit-0 advisory contract. The hook-utils.sh copy is bit-for-bit identical to lib/hook-utils.sh (verified via diff). One medium-severity portability issue with the awk CRLF fallback, several low-severity notes below.
Findings
Medium — awk CRLF fallback is not portable to POSIX/BSD awk
RS="\r?\n" as a regex multi-char record separator is a GNU awk extension. POSIX/BSD awk (including macOS /usr/bin/awk) treats RS as a single character — it reads only the first character of the string literal. In the awk string "\r?\n", \r is CR, so BSD awk silently sets RS=CR. On a LF-only input file (which is what the CRLF arm is converting), there are no CR characters to split on, so the entire file is treated as one record and emitted with a single trailing CRLF appended — producing wrong output.
In practice this is low-risk: normalize_eol_platform_crlf gates the CRLF arm on MINGW*|MSYS* only, and Git for Windows bundles both perl AND GNU awk, so the awk path is only reached on a Windows host that has Git Bash but somehow lacks perl. That combination is unusual. Still, the code comment says "portable fallback" and it isn't fully portable.
Suggested fix: gate the awk path on gawk explicitly, or replace with a portable sed equivalent:
# portable POSIX sed alternative:
sed 's/\r$//'"$file"| sed 's/$/\r/'>"$tmp"# or restrict to gawk:elifcommand -v gawk >/dev/null 2>&1;then
gawk 'BEGIN{RS="\r?\n"; ORS="\r\n"} {print}'"$file">"$tmp"fi
If $file_path contains ", \, or other JSON-special characters, this produces malformed JSON and hook::read_file_path's jq parse silently returns empty → exit 0 (no normalization, no error). In practice, test paths come from controlled mktemp output so this won't trigger. But using jq --arg would be both correct and self-documenting:
jq -nc --arg p "$file_path"'{"tool_input":{"file_path":$p},"tool_name":"Write"}'
Claude Code exposes a MultiEdit tool for applying multiple edits to a single file. The regex Write|Edit does not match MultiEdit (it matches on tool name, not substring). A file that receives multiple edits via MultiEdit would not be normalized.
This matches the pattern of all existing plugins in the repo (bash-lint, etc.), so it may be intentional or an ecosystem-wide gap. Worth confirming whether MultiEdit should be added as Write|Edit|MultiEdit.
Informational — Duration arithmetic assumes 6-digit microsecond precision in EPOCHREALTIME
local duration_ms=$(((e_s *1000000+10#$e_f- s_s *1000000-10#$s_f) /1000))
The arithmetic treats the fractional part as microseconds (6 digits). Bash documents EPOCHREALTIME as having "microsecond granularity" but does not guarantee exactly 6 fractional digits. On hosts where it differs, duration_ms would be off by orders of magnitude. This is a telemetry-only field and fail-open, so the impact is limited to bad timing data in the sink. Adding a comment about the 6-digit assumption would aid future maintainers.
Positives
Repo-agnostic and config-free — .gitattributes is the sole policy source; no hardcoded extension lists; no userConfig needed.
Advisory contract — always exits 0; documented clearly in comments, README, and code. Correct for a best-effort hook.
Security hardening — -- arg terminators on perl -pi, mv, rm, git check-attr; mktemp staging for in-place rewrites; jq --arg used throughout to prevent JSON/shell injection.
Stdin consumed once — INPUT=$(cat) + printf '%s' "$INPUT" | ... pattern correctly avoids double-drain of the pipe on Windows Git Bash.
hook-utils.sh is in sync — bit-for-bit identical to lib/hook-utils.sh (diff clean). CI enforcement mentioned in the source comment.
OS gating is correct — LF arm unconditional, CRLF arm Windows-only (MINGW*|MSYS*), with clear reasoning in comments and README.
Test coverage is solid — 27 assertions across LF normalization, no-op unspecified paths, kill switch, OS-gated CRLF, telemetry envelope schema (7 fields), action/status parity.
hooks.json, plugin.json, marketplace.json — all structurally consistent with existing plugins in the repo.
README is thorough: behavior matrix, requirements, install, kill-switch docs.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Cursor Bugbot did not complete (check skipped due to usage limit), so I cannot approve without its automated review signal. Human review is needed; no reviewers were assigned because no non-author reviewers are available in this repository.
Sent by Cursor Approval Agent: Pull Request Router and Approver
This is a re-review triggered by the sync merge commit (389aaf4) that brought in the desktop-notification plugin from main. The plugin code itself is unchanged from the prior review. The previous Claude review (comment above) already caught the main issues; this review confirms carryovers and adds one new architectural observation from the Codex reviewer.
Summary
Solid architecture. The core design is sound: .gitattributes-driven, advisory exit-0, security-hardened, well-tested. Four findings from the prior review remain open; one architectural design question (raised by Codex) is worth an explicit call on before merging.
hook-utils.sh sync confirmed: IDENTICAL to lib/hook-utils.sh ✓
Findings
Medium — awk CRLF fallback is GNU awk-specific (unaddressed from prior review)
RS="\r?\n" as a multi-character regex is a GNU awk (gawk) extension. POSIX/BSD awk (macOS /usr/bin/awk) treats RS as a single character — it reads \r (the first char of "\r?\n") and silently uses CR as the record separator. On a LF-only input file, no CR exists, so the entire file is treated as one record and emitted with a single trailing \r\n — producing corrupt output. The code comment calls this the "portable fallback" but it is not portable to the default awk on macOS or most BSDs.
The CRLF arm is Windows-only in practice, and Git for Windows ships gawk, so this doesn't trigger in the current gated path. But the fallback is dead code for all intents because perl is practically universal. Consider either gating on gawk explicitly or using a POSIX sed pipeline:
# POSIX-portable sed alternative:
sed 's/\r$//'"$file"| sed 's/$/\r/'>"$tmp"# Or gate on gawk:elifcommand -v gawk >/dev/null 2>&1;then
gawk 'BEGIN{RS="\r?\n"; ORS="\r\n"} {print}'"$file">"$tmp"fi
The Codex review raised this as P2. Worth surfacing explicitly for a decision:
eol=crlf in .gitattributes is a mandate for CRLF in the working tree on all platforms (not just Windows). When Claude writes a file on Linux/macOS, it writes LF; the hook then self-gates off and leaves the file as LF, violating the stated policy.
The current design rationale in the code is:
"git smudge already yields CRLF on a Linux/macOS checkout for eol=crlf paths; the only way such a file lands as LF on disk is a Windows Write that bypasses smudge."
This is accurate for checkout, but not for in-session edits: git smudge runs on checkout, not on every write. After Claude's Write/Edit, smudge does not re-run, so the file remains LF in the working tree until the next checkout/clean-reinstall.
The question for the author: Is this deliberate? If a repo on Linux has *.bat eol=crlf, this hook will leave .bat files as LF after Claude edits them. The advisory nature means it's non-blocking, but it does mean the hook is asymmetric: it actively enforces eol=lf on all platforms but deliberately skips eol=crlf enforcement on Linux/macOS.
If the intent is to be fully symmetric, remove the normalize_eol_platform_crlf gate from the crlf arm. If the intent is "only fix the common breakage vector (CRLF contaminating LF files)," the README's behavior matrix should call this out more explicitly.
Low — Test JSON construction fragile for paths with shell-special characters (unaddressed)
If $file_path contains ", \, or newline, this produces malformed JSON. In practice mktemp paths are clean, so it won't trigger. But jq --arg is the idiomatic, correct pattern — also consistent with how the production hook builds its own JSON:
jq -nc --arg p "$file_path"'{"tool_input":{"file_path":$p},"tool_name":"Write"}'
MultiEdit is a distinct tool name that does not match this regex. A file receiving multiple edits via MultiEdit would not be normalized. This is consistent with other plugins in the repo (all use Write|Edit), so it may be an ecosystem-wide gap rather than a plugin-specific bug. Either confirm MultiEdit should be out of scope and document it, or add it: "matcher": "Write|Edit|MultiEdit".
Low — CWE-377 parenthetical is inverted in meaning
# same-dir mktemp file (unpredictable name — CWE-377) then atomically mv.
This reads as though the unpredictable name is CWE-377, but CWE-377 is the vulnerability being prevented. The phrasing should say "CWE-377 prevented" or "mitigates CWE-377". Minor but potentially confusing to a reader auditing for security issues.
Informational — EPOCHREALTIME fractional-digit arithmetic (unaddressed from prior review)
local duration_ms=$(((e_s *1000000+10#$e_f- s_s *1000000-10#$s_f) /1000))
Arithmetic assumes exactly 6 fractional digits (microseconds). Bash documents this as the granularity but not as a guaranteed digit count. This only affects telemetry timing and is fail-open; a comment noting the assumption would aid future maintainers.
CLAUDE.md Compliance
Criterion
Status
Repo-agnostic (no hardcoded paths/names)
✓
Configurable without editing the plugin
✓ (.gitattributes is the sole authority; kill-switch via env)
Plugin-form-safe (${CLAUDE_PLUGIN_ROOT}, no ../ reach-outs)
Stdin consumed once — INPUT=$(cat) avoids pipe double-drain on Windows Git Bash.
OS gating is internally consistent with the documented design: LF arm unconditional, CRLF arm Windows-only.
Test coverage — 27 assertions including kill-switch, telemetry envelope schema (7 fields), action/status parity.
hook-utils.sh is in sync — confirmed bit-for-bit identical to lib/hook-utils.sh.
plugin.json, hooks.json, marketplace.json — structurally consistent with existing plugins.
Recommended action before merge: (1) Get a clear yes/no from the author on whether the CRLF cross-platform gap is intentional design (and if so, document it in the README behavior matrix). (2) Fix the GNU awk portability issue in normalize_eol_to_crlf. The remaining findings are low/informational.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Cursor Bugbot did not complete successfully (check skipped due to usage limit), so I cannot approve without its automated review signal. Human review is needed; no reviewers were assigned because no non-author reviewers are available in this repository.
Sent by Cursor Approval Agent: Pull Request Router and Approver
…mode preservation
Address Codex review findings on #38:
- P1: eol=lf alone is not proof of text — under '* text=auto eol=lf',
check-attr reports lf for binaries too. Mirror gitattributes semantics:
trust explicit 'text', skip '-text', NUL-sniff the first 8000 bytes for
text=auto (git's own detection window).
- P2: run the LF->CRLF arm on every OS, not just Windows — an LF write to
an eol=crlf path bypasses smudge and violates policy on any platform.
- P2: preserve the original file mode in the no-perl mktemp fallbacks via
cp -p before staging (chmod --reference is GNU-only).
Assess which prior findings were addressed by latest commit
Identify any remaining or new issues
Post comprehensive review findings
Summary
The latest commit (dfb3e29 — "binary guard, cross-platform crlf arm, fallback mode preservation") addresses every substantive concern from prior reviews and Codex. The three structural problems are now fixed cleanly and tested. Four low/medium findings from prior reviews remain open; the awk portability finding has increased in severity with the cross-platform change.
if awk 'BEGIN{RS="\r?\n"; ORS="\r\n"} {print}'"$file">"$tmp";then
This finding from prior reviews is now higher severity because of the cross-platform CRLF fix. Previously the CRLF arm was Windows-only, and Git for Windows ships GNU awk — so RS="\r?\n" (a GNU awk regex extension) was only reached in an environment that has it. Now the CRLF arm runs on every OS. On macOS (/usr/bin/awk is BSD awk) or any Linux distro with a minimal POSIX awk but no perl, the awk fallback is hit. BSD/POSIX awk treats RS as a single character — it reads only the first char of "\r?\n", which is \r (CR), and silently uses CR as the record separator. On an LF-only input file there are no CR bytes, so the entire file is treated as one record and emitted with a single trailing \r\n appended, producing corrupt output.
Perl is practically universal (macOS bundles it; most distros do too), so this is still a narrow failure path. But the comment calls this the "portable fallback," and it is not portable. The awk fallback is also untested in no-perl mode (Case 7 only exercises normalize_eol_to_lf; there is no analogous Case 8 for normalize_eol_to_crlf).
Suggested fix — replace with a POSIX sed pipeline that works identically on GNU and BSD:
# Portable: strip lone CR, re-add CR before each LF.if sed 's/\r$//'"$file"| sed 's/$/\r/'>"$tmp";then
mv -- "$tmp""$file"else
rm -f -- "$tmp"fi
$file_path containing " or \ produces malformed JSON; jq returns empty and the hook skips silently. In practice mktemp paths are clean, so this is not exploitable in the test suite. The production hook correctly uses jq --arg throughout; the test helper should be consistent:
jq -nc --arg p "$file_path"'{"tool_input":{"file_path":$p},"tool_name":"Write"}'
MultiEdit is a distinct tool name that does not match this regex. Consistent with all existing plugins in the repo, so this may be intentional. Worth an explicit decision: either confirm it is out-of-scope and add a note, or add MultiEdit to the matcher.
Low — CWE-377 comment phrasing is ambiguous (unaddressed)
# same-dir mktemp file (unpredictable name — CWE-377) then atomically mv.
The phrasing reads as "this IS CWE-377," but the unpredictable name is what prevents CWE-377. Should read "mitigates CWE-377" or "prevents CWE-377." Minor, but a future security auditor scanning for that CWE tag would misread the comment as flagging a vulnerability rather than documenting a mitigation.
Informational — EPOCHREALTIME fractional-digit count is undocumented (unaddressed)
local duration_ms=$(((e_s *1000000+10#$e_f- s_s *1000000-10#$s_f) /1000))
The arithmetic assumes exactly 6 fractional digits (microseconds). Bash documents EPOCHREALTIME with "microsecond granularity" but does not guarantee exactly 6 digits — a host returning fewer would produce wildly wrong telemetry timing. Impact is telemetry-only (fail-open). A short comment noting the 6-digit assumption would help future maintainers.
New Observation
The no-perl fallback test (Case 7) only covers normalize_eol_to_lf. There is no corresponding case for normalize_eol_to_crlf in no-perl mode, which means the awk portability bug (finding above) would not surface in CI. Adding a Case 8 that shadows perl and calls normalize_eol_to_crlf directly would catch this on any CI runner that runs with gawk vs. POSIX awk.
CLAUDE.md Compliance
Criterion
Status
Repo-agnostic
✓
Configurable without editing
✓
Plugin-form-safe
✓
No PII/secrets
✓
Versioned
✓
Security-reviewed
✓
Summary
Three of the four prior medium/P1/P2 findings are cleanly resolved. The implementation is well-structured: proper gitattributes semantics (text + eol together), NUL binary detection matching git's own window, idiomatic cp -p mode preservation, and symmetric coverage in the test suite. The one remaining blocker-candidate is the GNU awk portability issue in normalize_eol_to_crlf — the cross-platform change that correctly makes the CRLF arm symmetric also widens the blast radius of this fallback path to macOS/Linux.
Recommended before merge: Fix the awk fallback in normalize_eol_to_crlf (replace with POSIX sed pipeline or gate on gawk). The three low findings can be addressed at any time.
The reason will be displayed to describe this comment to others. Learn more.
Risk: medium. Cursor Bugbot did not complete successfully (check skipped), so I cannot approve without its automated review signal. Human review is needed; no reviewers were assigned because no non-author reviewers are available in this repository.
Sent by Cursor Approval Agent: Pull Request Router and Approver
The reason will be displayed to describe this comment to others. Learn more.
Preserve symlinks when rewriting EOLs
If a PostToolUse event names a symlink inside the project, the guard accepts it because -f follows symlinks, but this in-place rewrite replaces the symlink path with a regular file instead of updating the linked target; I verified this with a link.sh -> target.sh fixture, where the link was broken and the target kept its CRLF bytes. This can silently change repository topology for any symlinked text file governed by eol=lf (and the CRLF arm/fallback use the same replace-over-path pattern), so the hook should either skip symlinks or normalize the resolved target without replacing the link.
The reason will be displayed to describe this comment to others. Learn more.
Preserve missing EOF newline in CRLF fallback
When perl is not on PATH, this fallback uses awkprint, which terminates every output record with ORS; an eol=crlf file that intentionally has no final newline, such as printf 'abc', is rewritten as abc\r\n even though Git's own CRLF checkout preserves the missing EOF newline. That is a content change beyond line-ending normalization in the documented no-perl path, so the fallback should preserve whether the input ended with a newline.
…ss lint (#203)
## Summary
Closes decisions #38, #71, #72 (Decisions Log:
https://claude.ai/code/artifact/232ecdce-8316-4880-8c0a-dc3c7dcf3a63).
- **#38** `formatting-create-md-attribution-footer-drift`: `create.md`'s
PR-body heredoc now emits the `🤖 Generated with Claude Code` attribution
footer it documents elsewhere but previously didn't actually emit.
- **#71** `naming-work-items-checklist-template-staleness`:
`templates/checklist.md`'s `add` action reconciled to `add.md`'s actual
current behavior — `--force` skip, `--type` native-Issue-Type resolution
(org repos) vs. `type:` label (personal/non-org repos), `--agent-ready`
body template, `--recurring` title prefix — none of which the checklist
previously mentioned.
- **#72** `tooling-gov-commit-skill-composition-lint`: new `guardrails`
plugin hook `flag-commit-pr-skill-bypass.sh` — advisory
`PreToolUse`/`Bash` guard that flags direct `git commit` (missing the
canonical `-F -` + `--trailer` shape) or any `gh pr create`, nudging
toward this marketplace's own `/commit` / `/pull-request create` skills.
Gated on the consuming project actually having `source-control` enabled
in its own `.claude/settings.json` (fails quiet on uncertain state, per
the plugin's existing advisory-guard posture). Mirrors
`block-hook-bypass.sh`'s literal-stripping detection and the shared
telemetry envelope.
## Verification
- New hook's own test suite: 19/19 passed.
- `shellcheck` on both the hook and its test — clean.
- `guardrails` version bumped 0.3.2 → 0.4.0 (new toggleable guard),
README's guard table/kill-switch table/consumer-seams section updated to
match.
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrates the EOL-normalization PostToolUse hook into a repo-agnostic plugin. Bundles normalize-eol.sh via ${CLAUDE_PLUGIN_ROOT}; consumer conventions from .gitattributes/.editorconfig; kill switch HOOK_EOL_NORMALIZER_ENABLED; opt-in telemetry. Security-hardened (-- arg terminators + mktemp staging). 27 contract tests; plugin validate clean. Clean-tier wave Phase 1.
Note
Medium Risk
The hook mutates files on disk immediately after agent edits; guards and best-effort behavior limit damage, but wrong attribute resolution could still rewrite content outside intent.
Overview
Introduces a new eol-normalizer formatting plugin and lists it in the marketplace catalog.
On PostToolUse for
Write|Edit, the hook reads the edited path from stdin (with Windows Git Bash–safe single read), resolves the consuming repo root, and runsnormalize_eol_fileso working-tree endings match.gitattributes(eol=lf→ CRLF→LF,eol=crlf→ LF→CRLF on all platforms). Paths withouteol=,-text, or text=auto binaries (NUL sniff) are left alone. The hook is advisory (always exit 0), disable viaHOOK_EOL_NORMALIZER_ENABLED, and optionally emits telemetry through the sharedhook-utilscopy bundled in the plugin.The plugin ships
normalize-eol.sh(perl withtr/awk+mktempfallbacks that preserve file mode),hooks.jsonwiring, README, and a black-boxeol-normalizer.test.shsuite covering LF/CRLF arms, kill switch, binary/-textguards, and telemetry envelopes.Reviewed by Cursor Bugbot for commit dfb3e29. Bugbot is set up for automated code reviews on this repo. Configure here.