Conversation
Validate Commits ran the trailer check as a pipeline under pipefail with grep -q. grep -q exits on its first match, so for a commit message larger than the pipe buffer git was still writing, took SIGPIPE, and returned 141. pipefail made that the pipeline result, and the if turned a real match into a miss. The banned trailer passed. Use grep -c with a numeric test. grep -c reads to end of input, so the writer always finishes and no SIGPIPE arises. It keeps grep's own ERE engine and -i semantics, unlike a bash =~ rewrite, so the pattern behaves identically. Verified on four real commits: a 3 MB message carrying a bot trailer reports clean under the old construct and flagged under the new one, while a small bot trailer, a large human trailer, and a large clean message are unchanged. Closes #486
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes the fail-open reported in #486, matching z-shell/.github#596.
The defect
.github/workflows/commit-lint.ymlcarries the same construct reported inz-shell/.github#587. The
Validate Commitsjob matches the disallowed trailer as a pipeline underset -o pipefail:grep -qexits on its first match. For a commit message larger than the pipebuffer,
gitis still writing, takesSIGPIPE, and returns141.pipefailpromotes that to the pipeline result, so the
ifreads a real match as nomatch and the banned trailer passes.
Evidence
Four real commits evaluated under both constructs:
grep -qgrep -cCo-authored-byOnly the second row changes, which is the defect.
Fix
Use
grep -cwith a numeric test.grep -creads to the end of its input, sothe writer always finishes and no
SIGPIPEarises, and it keeps grep's own EREengine and
-isemantics rather than swapping in bash[[ =~ ]]matching.grep -cwas chosen over a[[ $message =~ $pattern ]]rewrite deliberately.Both stop the fail-open, but
[[ =~ ]]swaps grep's ERE engine for bash's andneeds
shopt -s nocasematchto keep the-ibehaviour. The pattern contains[[:space:]]and\[bot\], and a silently different match here is worse thanthe bug being fixed.
actionlintis clean.Why it matters here
This repository is one of the two AGENTS.md names as enforcing the bot and
AI-agent
Co-authored-byban in CI. A guard that fails open is the same classof problem as z-shell/.github#575, where the enforcement existed but never
actually ran.
Closes #486