Re-Sign Rewritten Commits on Attach

No, not on the path this issue is about.

session attach handles a commit in one of two ways:

• Unpushed commit, rewritten with a trailer (the issue's case). There's no author check. rewriteWithTrailer (cmd/entire/cli/attach_confirm.go:192) copies each commit's original author and committer (name, email and date) onto its replay through GIT_AUTHOR_* and GIT_COMMITTER_* (lines 212–219). It does this whoever made the commit. The chain includes every commit after the target up to HEAD, so a teammate's commits on your branch get rewritten too, and nothing compares them to you. The warning (rewriteWarning, line 142) only checks for a signature.
• Already-pushed commit, linked in the checkpoint instead. This is the only author check, and it only warns. attachWarning (cmd/entire/cli/attach.go:583) and reportLinkedCommit (attach.go:1027) compare your GetGitAuthor email to the commit author's email. If they differ, they tell you the link "counts only when the commit's author attaches it." They don't block the link.

Why it matters for the fix they propose: adding -S to the replay would sign with your key while the committer field still names the original committer. That causes two problems for a commit someone else made:

1. You'd sign their commit. GitHub marks a commit verified only when the signing key matches the committer's email, so the result would show as unverified. That defeats the purpose on a ruleset that requires signed commits. It would also put your signature on their work.
2. Git might not find a key. If user.signingkey is unset, git picks the key from the committer identity. Because we override that identity with the original committer, git would look up their key: it either fails, or uses their key if it happens to be in your keyring.

So the PR should add -S only when the original commit was signed and its committer is you (or your signing identity). Otherwise it should keep today's "Signed commits lose their signatures" warning for that commit. In practice most chains are entirely your own commits, so the fix still covers the common case.

I haven't commented on the issue. I can draft that condition for the reporter if you want.

No, I didn't find another issue, PR or trail about the author in session attach.

• GitHub issues: none besides #2974. The searches also covered closed issues. The closest earlier work is the merged PR #2672, which added the "Signed commits lose their signatures" warning, as the reporter mentions.
• Open PRs: six open PRs touch attach (#2781, #2642, #2299, #2536 and two others). None of them is about the author, the committer or signing.
• Trails: none either. One is close by: trail 2481, "Fix attach tests for git 2.56", which is open. It changes only attach tests: git 2.56 made a push by URL update remote-tracking refs, which broke TestAttachCommit_FetchesBeforeDecidingAHeadIsUnpushed. The reporter is on git 2.56 too, so a PR from them with a new attach test could hit the same failure until 2481 lands.

Matches on "author" that aren't about attach:

• Issue #2718 asks for an --author filter on entire checkpoint list.
• PR #2524 makes doctor and status link unattributed authors on reserved hosts.
• Trail 2331 is about which sessions get linked to an identified committer's commit, not about attach.

I only checked the 100 most recently updated cli trails, so an older trail on this could still exist.

Yes. The safest way is to compare the key that signed the original commit with the key that signs the replay, rather than trying to work out from your git config which key is "yours".

Reading the signing key from git works. git log --format='%G? %GK %GF' gives the key that made the signature. I checked this for SSH signing with git 2.50 in a scratch repo, using one commit signed with my test key and one signed with a different key:

• Without gpg.ssh.allowedSignersFile, git reports nothing (N, all fields empty) and prints an error. Most people who sign with SSH never set that file, so we'd pass a temporary one with -c gpg.ssh.allowedSignersFile=<tmp>.
• With the file set, git reports the fingerprint: SHA256:f4ny… on my commit and SHA256:Sn3u… on the other one. %G? was G for my commit, whose key was in the file, and U for the other.
• ssh-keygen -Y check-novalidate -n git also works without any config. It checks the signature and prints the fingerprint of the key that made it.

Matching that against user.signingkey is the hard part:

• SSH: the setting can be a file path or a key:: literal. It can also be unset, with gpg.ssh.defaultKeyCommand picking a key, for example the first key in ssh-agent or 1Password.
• OpenPGP: the setting can be a short ID, a long ID, a fingerprint, an email or a !-suffixed subkey. %GF reports the subkey's fingerprint and %GP the primary key's, so the comparison has to try both. If the setting is unset, git looks up the key by the committer identity, and we override that identity with the original committer. I couldn't test this format because gpg isn't installed here.
• x509: another format with its own rules, also untested.

Simpler: sign first, then compare. For each commit that was signed before:

1. Replay it with commit-tree -S.
2. Read the key fingerprint from both the old and the new commit, using the temporary allowed-signers file for SSH.
3. If the two keys match, keep the signed replay. The original was signed with a key you can sign with, so it's yours whatever the format or config.
4. If they don't match, replay that commit again without -S and keep today's "loses its signature" warning for it.

Throwing a commit away costs nothing, because rewriteWithTrailer only writes objects until the final update-ref.

The cost is that a commit someone else signed still triggers your signer before we discard the result, which can mean a pinentry prompt or a 1Password approval. To avoid that, first skip -S when the original committer's email isn't yours, using the same GetGitAuthor comparison attach already does for linked commits. The email check is only a cheap filter; the key comparison decides.

Two parts are untested: OpenPGP and x509. With OpenPGP, %GK should still report the key ID even when the other person's public key isn't in your keyring (%G? = E), which is enough to see it isn't your key, but I haven't confirmed it.

Do you want me to write this up as a suggestion for the reporter on #2974? I won't post anything without your go-ahead.

I built it on a new branch, soph/attach-keep-signatures, in three commits. The attach tests, lint and a Windows vet pass are green. The full mise run check is still running in the background, and I'll report back when it finishes.

1. 528c76c566 — attach: replay each rewritten commit through one commitTree helper. Moves the commit-tree call out of the loop into a helper so it can run twice for one commit. No behavior change.

2. 374059051e — attach: sign rewritten commits again when they carried your signature. A replayed commit is signed again with commit-tree -S, which uses your own signing config (gpg.format, user.signingkey, the signer program). That only happens when:

   • the original commit was signed,
   • its committer email is yours,
   • and git reports the same key for the new signature as for the original. For SSH signatures, git only reports the key when an allowed-signers file is set, so the comparison passes /dev/null as one.

   In every other case, including when signing fails, the commit is replayed unsigned as before. Attach then prints warning: commit X (was Y) is no longer signed: <reason>.

3. 5013386d4d — attach: say up front which signed commits keep their signature. The confirmation used to say "Signed commits lose their signatures." It now says your signed commits are signed again, and lists any signed commits someone else committed, which still lose their signatures.

Tests: I added five tests that use real SSH keys made by ssh-keygen, and they skip when ssh-keygen isn't installed. They cover:

• your signature being kept on both the target and a commit after it,
• someone else's commit not being signed, with your signer never asked,
• a commit you made but signed with a key you no longer use losing its signature,
• signing failing and the commit being rewritten unsigned,
• the new confirmation text.

I removed each check in turn (the committer email, the key comparison, -S), and at least one test failed every time.

Not tested:

• OpenPGP and x509 signing: gpg isn't installed here.
• Windows: I couldn't run the /dev/null path for SSH key lookup there. If it doesn't work, git reports no key and the commit is left unsigned with the warning, which is today's behavior.

Nothing is pushed, and I haven't commented on #2974. The reporter offered to send a PR themselves, so before I push, do you want to open this as our PR or point them at the branch?

Yes, both: CI now runs the OpenPGP and x509 tests on every PR and fails if they would skip, and I ran them locally in a Docker container. All three formats pass. That's a fourth commit on soph/attach-keep-signatures, still not pushed.

06c1eefb90 — attach: test re-signing with OpenPGP and X.509 keys too, and require them in CI

• Tests: the signing tests moved to cmd/entire/cli/attach_signing_test.go, and each one runs once per format. The keys are throwaway and never touch your own keyring:

  • SSH: ssh-keygen.
  • OpenPGP: gpg, in a fresh GNUPGHOME per test.
  • x509: self-signed certificates from gpgsm, also in a fresh GNUPGHOME.

  The gpg agent is stopped when each test ends.

• One new test: OpenPGP with user.signingkey unset, where git picks your key from the committer identity. Your signature is kept.

• CI: the core test job installs any of ssh-keygen, gpg and gpgsm that's missing. It also sets ENTIRE_TEST_REQUIRE_COMMIT_SIGNERS=1, so a missing tool fails the job instead of quietly skipping. This copies how ENTIRE_TEST_REQUIRE_CODEX_APP_SERVER already works in ci.yml.

• Docs: docs/development/testing.md has a short section on these tests and how to run them in a container.

Results:

• In Docker (Debian, git 2.39, GnuPG 2.2), with the require flag set: all 14 subtests pass and none skip. When I removed the key comparison, the SSH, OpenPGP and x509 subtests all failed.
• On this Mac: SSH passes and the other two formats skip.
• Not yet run on the real CI image, which has newer git and GnuPG; we'll see that on the first CI run. I also couldn't confirm whether that image already has gpgsm. If it doesn't, the install step adds it.

mise run check failed locally, but not because of this branch. Two tests that don't touch attach fail on a clean checkout of main too:

• TestHTTPS_PushFailsWithoutToken hangs until it times out.
• TestOpenCodeSeedRepoPlantsDeps fails with node_modules is not a symlink.

Lint is clean, and every attach test passes.

Do you want me to push and open the PR, or should we point the reporter of #2974 at the branch first?

I opened trail #2692, "Keep your signature on commits session attach rewrites": https://entire.com/gh/entireio/-/trails/2692

• Branch: soph/attach-keep-signatures is pushed with all four commits; its change on the trail is cli/1741.
• Description: what was wrong, the three conditions for signing a rewritten commit again, the commit list, the tests, and what is and isn't verified. That includes the newer git and GnuPG on the CI image, and Windows, neither of which has run yet. The description starts with "Fixes #2974."
• Before the push: lint on the final tree was clean.
• Findings: none yet. The review may not have run, so check again before calling it clear.

I haven't commented on #2974. The reporter offered to send their own PR, so you may want to tell them about this branch.

I've fixed all five Copilot comments on PR #2978 locally but haven't committed or pushed yet. mise run check is still running, and I'll commit and push once it finishes.

Failing CI: the failure isn't caused by this branch. test-integration (c) failed in TestMultiPushURL_Branch_RepeatedPushesKeepBothURLsInSync with cannot exec .../pre-push: Text file busy. That's a Linux race where a script gets executed while a forked process still has it open for writing, and it's in multi-push-URL code the branch doesn't touch. The test job is just the summary step that turns red when any job fails. test-core, which runs the signing tests on the CI image with all three tools required, passed. So did test-windows, but I don't know whether the SSH signing tests actually ran there or were skipped.

Copilot's comments: all five were real problems:

One cost: re-signing now takes one extra signature per attach, for the throwaway commit. Anyone with a hardware key touches it one more time.

New tests: there's a test for a tampered signature in each format, and one for committer.email differing from user.email. The keep-signature test now runs with log.showSignature on. The other-key test now also checks that no commit with the target's files was ever signed by your key.

Verified:

• All signing tests pass in Docker (Debian, all three formats).
• I undid each fix in turn, and each time a test failed, in every format.
• Lint is clean, and the Windows go vet passes.

There are no trail findings on cli/1741 yet. After the push I'll check them again and resolve Copilot's threads without replying.

I fixed all five Copilot comments. The CI failure has nothing to do with this branch. The fix is pushed as b02528d339, and CI is running on it now.

Failing CI: test-integration (c) failed in TestMultiPushURL_Branch_RepeatedPushesKeepBothURLsInSync with cannot exec .../pre-push: Text file busy. That's a Linux race over a hook file in multi-push-URL code this branch doesn't touch. The red test job is only the summary step, which fails when any job fails. test-core, which runs the signing tests on the CI image with all tools required, passed.

Copilot's comments: all five were right, and all are fixed in that commit:

• Signing before checking the key: a commit could end up signed by your key and left behind in the repo even when the rewrite threw it away. Attach now first finds which key you sign with by signing a throwaway commit with no files in it, once per committer. It only signs a real commit once that commit's key is known to match. The cost is one extra signature per attach, so anyone with a hardware key touches it once more.
• Tampered signatures passed: git reports the key of a bad signature too, so a commit with a forged signature could get re-signed with your key. Attach now also requires git to report the signature as good. A tampered commit is rewritten unsigned, with "its signature can't be verified".
• Your identity: it was read from user.email only. It now comes from git var GIT_COMMITTER_IDENT, so committer.email and GIT_COMMITTER_EMAIL count too.
• log.showSignature=true: with that set, git printed the signature check into the key lookup, so the keys never matched and your signatures were dropped. The lookup now passes --no-show-signature.
• CI tool check: I confirmed that command -v a b c exits 0 if any one tool exists. The step now checks each tool separately.

Tests and checks:

• New tests cover a tampered signature in each format and a committer.email that differs from user.email. The keep-signature test now runs with log.showSignature on. The other-key test now also checks that your key never signed the commit's content, even in a commit attach discarded.
• All signing tests pass in Docker with SSH, OpenPGP and X.509. Undoing any one of the four code fixes makes a test fail in every format.
• Lint is clean and the Windows go vet passes.
• mise run check caught that the throwaway commit's message, "entire attach: …", tripped the deprecated-command test, so I reworded it. The one remaining failure there, TestOpenCodeSeedRepoPlantsDeps, also fails on clean main.

Still open:

• Resolving Copilot's five threads on GitHub was blocked by the permission check, so they're still open. You can resolve them in the PR, or allow the gh api graphql resolveReviewThread call and I'll do it.
• Trail cli/1741 shows no findings yet, but the review may not have run on the new commit. I'll check again once CI finishes.

I resolved all five of Copilot's threads on PR #2978 without replying to any of them.

CI on the new commit is green, including test-core (which runs the signing tests with all three formats required), test-integration (c) and test-windows. Only "Entire Gates" is still pending.

Trail cli/1741 still lists no findings, but with that gate unfinished the review may not have run on this commit yet.

PR #2978 now has a new description and is ready for review: https://github.com/entireio/cli/pull/2978

• Title: changed from the branch name to the trail's title, "Keep your signature on commits session attach rewrites".
• Description: it now includes today's review fixes:
  • the signature must check out before re-signing;
  • your signing key is found before anything is signed, so your key never signs a commit attach then throws away;
  • your identity now honours committer.email;
  • --no-show-signature is passed;
  • the new tests and the fifth commit are listed.
• Kept: the hidden entire-shadow-pr marker that lets Entire close the PR when the trail closes.
• Removed: Cursor's old summary block, which described the first commit. Cursor should add a fresh one on its next review.

The trail #2692 description is still the original text, without these fixes. Do you want me to update it to match the PR?

Here's trail #2692: https://entire.com/gh/entireio/-/trails/2692

Nothing new since Friday. The PR and the trail are both waiting for a review.

PR #2978:

• No new reviews, comments or threads. Copilot's five threads are all resolved and nothing has been added since.
• It's ready for review and mergeable, with "review required".
• Every CI check passes except Entire Gates, which fails only because there's no approval yet.

Trail #2692 (cli/1741), gates on the latest commit b02528d3:

• ✗ approvals: no reviewer has approved it yet. This is the only thing failing.
• ✓ checks: all 22 passed.
• ✓ findings: the agent review found no blocking issues, and finding list shows none.
• – up to date: 26 commits behind main. It isn't blocking and there are no conflicts.

The trail description is still the original text. It doesn't mention the review fixes, and it says the change hasn't run on the CI image, which it now has. Do you want me to copy the PR description over to it?

The reporter (rennf93) left one new comment on #2974 on Oct 30. It's no longer about signing: it's about the commit ID changing. Their agents quote the SHA they just committed, attach rewrites the commit, and the quoted SHA no longer points at anything on the branch. They've had to tell every agent to use the SHA "the hook prints after the commit". They ask for two things.

1. Print the old and new SHA for every rewritten commit. This is only partly there today. rewriteWithTrailer (cmd/entire/cli/attach_confirm.go:276) prints:

That gives the target's old and new SHA, but only the old HEAD. A commit replayed in the middle of the chain is reported only if it loses its signature. The full old→new list is already built in that loop (pairs, which goes to PostRewrite), so printing a line per commit is a small change.

2. Keep the link outside the commit so the signature and the ID both survive. This already exists, but only for pushed commits. For those, attach leaves the commit alone and records the link in the checkpoint, keyed by the commit's SHA. That's essentially the second option they describe. What's missing is a way to choose that path for an unpushed commit. It was kept out on purpose, and the code says why (attach.go:577 and :584):

• the CLI can't verify who recorded a link, because anyone who can push checkpoints can name any commit;
• a recorded link counts only when the commit's author is the one attaching it.

So an opt-in such as --no-rewrite could reuse the existing path. The cost is that the commit gets a link that's easier to fake than a trailer, which is a product decision rather than just a code change.

Worth asking them: commits made during a session Entire is tracking get the trailer from the prepare-commit-msg hook as they're made, with no rewrite. Attach only rewrites when it runs after the commit. If their agents need attach after every commit, the real gap may be that those commits aren't getting the trailer at commit time, for example because the agent isn't hooked. That would be the thing to fix first.

My recommendation:

• Add the per-commit SHA lines (point 1) to this PR or a small follow-up.
• Treat point 2 as a separate decision.
• Ask them which agent they use and why attach runs after each commit.

I haven't replied on the issue. Should I draft a reply, add the per-commit SHA output to PR #2978, or both?

rennf93 tested the PR on macOS with GnuPG installed and found a real bug. The fix is made locally; mise run check is running and I'll commit when it's done.

Their report: SSH passes, but every OpenPGP and X.509 test fails while generating the test key, before attach even runs:

The cause is useGnuPGHome, which used t.TempDir(). On macOS that path includes the test name, so the gpg-agent socket path reached 138 bytes, and macOS allows 104. Formats are skipped only when the tool is missing, so go test ./cmd/entire/cli fails on any Mac with GnuPG installed, with or without the CI variable. CI and my Docker runs are Linux, where the limit is 108 and the path is shorter, so neither caught it.

The fix:

• I used their suggestion: the home comes from os.MkdirTemp("", "gpg"). On this Mac that puts the socket path at 74 bytes, and the longest socket name at 82.
• MkdirTemp also creates the directory with 0700 permissions, which stops gpg's "unsafe permissions" warning.
• One change from their snippet: the cleanup reports a failed RemoveAll instead of discarding the error, because lint flags _ =.
• The order of cleanups still stops the agent before its home is deleted.

Checked:

• All 8 signing tests pass in Docker with all three formats, and no temp homes are left behind.
• I couldn't reproduce the failure or confirm the fix on macOS, because gpg isn't installed here. They ran this change on macOS and all 8 tests passed in every format.

They also confirmed independently that removing -S fails the "keeps" tests in every format, while the "should be unsigned" tests still pass. They only ran the tests; they haven't tried attach with their real key.

I haven't replied on the PR. Once this is pushed, do you want me to draft a short thank-you that says their fix went in?

The first mise run check failed on lint: usetesting objects to os.MkdirTemp in a test and wants t.TempDir(). Here the short path is the whole point of the fix, so I added a //nolint:usetesting that says why, as bench_enable_test.go:29 does. Lint and the full check are running again, and I'll commit when they pass.

I pushed the fix for the macOS bug rennf93 found as fddadcef34, "attach: keep the test GnuPG home short enough for the agent's socket". The commit message credits their report.

• Fix: the test GnuPG home now comes from os.MkdirTemp instead of t.TempDir(). On this Mac that keeps the gpg-agent socket path at 74 bytes, under macOS's 104-byte limit.
• Lint: the linter wants t.TempDir() in tests, so the line has a nolint comment saying why it's needed here.
• Checks: lint is clean. All 8 signing tests pass in Docker with all three formats. mise run check fails only on TestOpenCodeSeedRepoPlantsDeps, which fails on clean main too.
• Not verified by me on macOS: gpg isn't installed here. The reporter ran this exact change on macOS and all three formats passed.
• Trail cli/1741: no findings right after the push. CI hasn't finished on the new commit yet.

I haven't replied to their comment. Do you want me to draft a short thank-you saying their fix is in fddadcef34?

I've fixed all four trail findings on cli/1741 locally; nothing is committed yet. mise run check is running, and once it passes I'll commit, push and resolve the findings.

1. Medium: your own commits lost their signature after you added a signing subkey. Confirmed in Docker. A newer OpenPGP subkey changes %GK (the key ID) and %GF (the signing key's fingerprint), but %GP (the primary key's) stays the same. The check now compares %GP when there is one, and %GF otherwise; for SSH and X.509 that's already the full fingerprint. A new test adds a subkey between signing and attach.
2. Low: control characters in the committer email reached the output. Confirmed: git keeps a raw ESC byte in the email. The reason now quotes it with %q, as describeCommit already does.
3. Low: the confirmation was decided from the committer email alone.
   • The confirmation now runs the same checks that need no signing: that you committed it and that its signature is good. It lists every signed commit that loses its signature, with the reason.
   • For your other commits it says they're signed again "if your signing key made their signature; otherwise they lose it".
   • I didn't move the key comparison before the prompt. That would mean signing the throwaway commit before you've agreed, which could ask for a passphrase or a hardware-key touch.
   • When git has no email for you, the reason is now "git has no email for you to compare its committer with", instead of calling your own commit someone else's.
4. Low (nits):
   • replayCommit is split into checkSignature and signAgain, so no err hides another and the signing-failure text is built once.
   • signingProbe is now a struct with a pointer receiver, so an empty one no longer panics on its first use.
   • The PR description said removing the signature check fails a test in every format; that wasn't true for SSH before. With the new key lookup, it now is.

Verified:

• The attach tests pass locally (SSH only on macOS) and in Docker (all three formats).
• I undid each of the eight checks one at a time (the four from before, plus the subkey match, the quoting, the no-email case and -S), and each time at least one test failed.
• My first version broke SSH on macOS, and the local run caught it: when %GP was empty, trimming the output dropped a field. The fields are now separated with NUL bytes.

The PR description now has some outdated details: it says %GK and lists only five commits. Updating it to match: