entireio / entireio/cli

`AppendCheckpointTrailer` can emit a trailer the final-block parser (and git) reject

Open
#2,256 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
5.1k
Forks
475
Avg merge
1d 11h
Merged PRs (30d)
178

Description

Summary

trailers.appendTrailerLine (the writer behind AppendCheckpointTrailer and AppendOPFAppliedTrailer) and trailers.finalTrailerBlock (the reader behind the strict ParseCheckpointFromFinalTrailerBlock / ParseAllCheckpointsFromFinalTrailerBlock parsers introduced in #2253) implement two different grammars for "is this paragraph a trailer block". When a commit message ends with an indented checkpoint-shaped line, the writer joins a real trailer onto that paragraph without a separating blank line, producing a message in which neither our strict readers nor git itself can see the trailer that was just appended. The CLI reports success while every downstream hook silently ignores the commit.

The reader's grammar matches git's; the writer is the buggy side.

The grammar divergence

Writer (cmd/entire/cli/trailers/trailers.go, appendTrailerLine): to decide whether the message already ends in a trailer block, it takes the last non-comment line and tests IsTrailerLine(strings.TrimSpace(line)). The TrimSpace means an indented line like ␣␣Entire-Checkpoint: A — which is not a trailer to git — passes the check. It then walks upward, again trimming each line, until it finds a blank line, and concludes "trailer block present, join directly".

Reader (finalTrailerBlock): isolates the final paragraph and accepts it only if every line is either a real trailer line (IsTrailerLine on the raw line — leading whitespace fails the ^[A-Za-z][A-Za-z0-9-]*: regex) or an indented continuation of a preceding trailer. An indented line at the top of the paragraph has nothing to continue, so the entire paragraph is rejected and the parsers return nothing. This intentionally mirrors git, which treats an indented line after a trailer as a continuation of that trailer's value, never as a new trailer.

So the writer's membership test is strictly looser than the reader's, and the looseness is load-bearing: it changes where the writer places the new trailer.

Reproduction

Start from a commit message whose final paragraph is an indented checkpoint-shaped line — for example one pasted from attach's own non-interactive hint, which prints the trailer with a two-space indent (Copy to your commit message to attach:\n\n Entire-Checkpoint: <id>):

Message

Body.

  Entire-Checkpoint: <A>

AppendCheckpointTrailer(msg, B) classifies the final paragraph as an existing trailer block and joins without a blank line:

Message

Body.

  Entire-Checkpoint: <A>
Entire-Checkpoint: <B>

Verification against git, which is the authority here:

$ git interpret-trailers --parse < joined.txt
$                                              # zero trailers — git rejects the whole paragraph
$ git interpret-trailers --parse < separated.txt
Entire-Checkpoint: <B>                         # same message with a blank line before B

ParseCheckpointFromFinalTrailerBlock returns the same verdicts as git on both shapes. A writer/reader roundtrip test (AppendCheckpointTrailerParseAllCheckpointsFromFinalTrailerBlock) fails on the space- and tab-indented cases.

User-visible failure chain

  1. promptAmendCommit (cmd/entire/cli/attach.go) builds the new message with trailers.AppendCheckpointTrailer, runs git commit --amend, and prints Amended commit <hash> with Entire-Checkpoint: <B>.
  2. PostCommit (cmd/entire/cli/strategy/manual_commit_hooks.go) parses HEAD's message with ParseCheckpointFromFinalTrailerBlock, finds nothing, and takes the no-trailer path: the commit is treated as mid-turn — no checkpoint linkage, no condensation. Same for the commit-msg validation and parseCheckpointFromCommitMessageFile, so the prepare-commit-msg path (addCheckpointTrailer delegates to the same writer) is equally affected.
  3. Attach's idempotence check (ParseAllCheckpointsFromFinalTrailerBlock over HEAD before amending) also can't see B, so re-running attach appends the same trailer again into the still-rejected paragraph. The failure is self-reinforcing rather than self-correcting.

The net effect inverts #2253's intent at this one seam: that PR ensures body text can't grant checkpoint authority; this bug makes a legitimately granted trailer lose authority while reporting success.

Secondary inconsistency (same root cause)

A valid trailer block whose last line is an indented continuation, e.g.

Message

Signed-off-by: Test User <test@example.com>
 continuation text

makes the writer conclude "no trailer block" (the trimmed last line isn't a trailer) and insert a blank line, splitting what git considers one block into two. B is still recognized (it forms its own final block), so this is cosmetic — but it falls out of the same divergent grammar and is fixed by the same change.

AppendOPFAppliedTrailer shares appendTrailerLine, so it carries the same latent shape; in practice v1 checkpoint messages are generated entirely by Entire's own formatters and can't naturally end in an indented pseudo-trailer, so no OPF-specific behavior change is expected beyond inheriting the fix.

Proposed fix

Single source of truth for the grammar:

  1. Extract finalTrailerBlock's classification loop into a shared isTrailerBlock(lines []string) bool: at least one trailer line, every line either a trailer or an indented continuation of a preceding trailer.
  2. Rewrite appendTrailerLine to collect the final non-comment paragraph as raw lines (comment-line skipping preserved for the prepare-commit-msg template case) and join only when isTrailerBlock(paragraph) holds and a blank line separates the paragraph from the content above. The blank-line requirement preserves the existing guard that keeps a trailer-shaped subject line (fix: bug matches the trailer regex) from having the trailer glued onto it.

For the reproduction above, B then starts a fresh final paragraph:

Message

Body.

  Entire-Checkpoint: <A>

Entire-Checkpoint: <B>

git and the strict readers both see B; the indented A remains a body occurrence, i.e. a discovery-only candidate for the permissive squash-history parsers — exactly the contract #2253 established. No changes to the permissive parsers or squash-resume behavior are needed.

Validated locally: new regression cases in TestAppendCheckpointTrailer plus a writer/reader roundtrip test fail before the change and pass after; full unit (10k+), integration, and Vogon canary suites green; gofmt/golangci-lint clean.

Related

  • #2253 introduces the strict final-block readers this writer must agree with; it does not change the writer. On current main the mismatch only affects HasOPFApplied (theoretical, since v1 messages are machine-generated); once #2253 merges, the checkpoint attach/PostCommit chain above becomes reachable, so this should land as a follow-up to it.
  • Distinct from #2255 (body text forging authority — the inverse failure) and from #2168/#2176 (shadow-commit subject forgery in FormatShadowCommit).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in cmd/entire/cli/trailers/trailers.go, comparing appendTrailerLine with finalTrailerBlock and the strict parsers. Run TestAppendCheckpointTrailer and the writer/reader roundtrip cases, including space- and tab-indented lines. Done means appended trailers are recognized consistently by the strict readers and git, while trailer continuations remain in one block.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, go
Domain
cli, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.