Skip to content

fix(ci): stop the trailer guard failing open on large messages - #487

Merged
ss-o merged 1 commit into
nextfrom
bug-486
Sep 2, 2026
Merged

fix(ci): stop the trailer guard failing open on large messages#487
ss-o merged 1 commit into
nextfrom
bug-486

Conversation

@ss-o

@ss-o ss-o commented Sep 2, 2026

Copy link
Copy Markdown
Member

Fixes the fail-open reported in #486, matching z-shell/.github#596.

The defect

.github/workflows/commit-lint.yml carries the same construct reported in
z-shell/.github#587. The
Validate Commits job matches the disallowed trailer as a pipeline under
set -o pipefail:

git show -s --format='%B' "$sha" | grep -qiE "$DISALLOWED_TRAILER_PATTERN"

grep -q exits on its first match. For a commit message larger than the pipe
buffer, git is still writing, takes SIGPIPE, and returns 141. pipefail
promotes that to the pipeline result, so the if reads a real match as no
match and the banned trailer passes.

Evidence

Four real commits evaluated under both constructs:

Commit message grep -q grep -c
Small, bot trailer FLAGGED FLAGGED
3 MB, bot trailer clean FLAGGED
3 MB, human Co-authored-by clean clean
3 MB, no trailer clean clean

Only the second row changes, which is the defect.

Fix

Use grep -c with a numeric test. grep -c reads to the end of its input, so
the writer always finishes and no SIGPIPE arises, and it keeps grep's own ERE
engine and -i semantics rather than swapping in bash [[ =~ ]] matching.

grep -c was chosen over a [[ $message =~ $pattern ]] rewrite deliberately.
Both stop the fail-open, but [[ =~ ]] swaps grep's ERE engine for bash's and
needs shopt -s nocasematch to keep the -i behaviour. The pattern contains
[[:space:]] and \[bot\], and a silently different match here is worse than
the bug being fixed.

actionlint is clean.

Why it matters here

This repository is one of the two AGENTS.md names as enforcing the bot and
AI-agent Co-authored-by ban in CI. A guard that fails open is the same class
of problem as z-shell/.github#575, where the enforcement existed but never
actually ran.

Closes #486

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
@ss-o
ss-o merged commit 40d8d5e into next Sep 2, 2026
8 checks passed
@ss-o
ss-o deleted the bug-486 branch September 2, 2026 23:07
@ss-o ss-o mentioned this pull request Sep 3, 2026
13 tasks
@github-actions github-actions Bot mentioned this pull request Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant