From b0dd2b160a090aa6c5e448a545bc3fdbcbee604b Mon Sep 17 00:00:00 2001 From: fullsend-code <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Sun, 30 Aug 2026 16:31:53 +0000 Subject: [PATCH 1/4] fix(#6688): add per-commit bot email detection for DCO classification Add IsBotCommitEmail() to the forge package, providing a canonical, tested function that identifies GitHub App bot noreply emails (+[bot]@users.noreply.github.com). This enables per-commit DCO classification instead of branch-wide operations that destroy valid human Signed-off-by trailers on mixed-author branches. The root cause of #6688 was post-fix validation using git filter-branch --msg-filter to strip ALL Signed-off-by trailers from ALL commits on a branch. On PR #6383, this destroyed six valid human DCO attestations while trying to enforce the bot-only rule that bot commits must not carry Signed-off-by. Changes: - internal/forge/forge.go: add IsBotCommitEmail() with compiled regex matching the GitHub App bot noreply pattern - internal/forge/signoff_test.go: add table-driven tests covering bot emails, human emails, and edge cases - docs/contributing/bot-identities.md: add per-commit DCO classification section with Go and shell examples, and explicit guidance against branch-wide git filter-branch Related to #6688 --- docs/contributing/bot-identities.md | 40 +++++++++++++++++ internal/forge/forge.go | 33 ++++++++++++++ internal/forge/signoff_test.go | 69 +++++++++++++++++++++++++++++ 3 files changed, 142 insertions(+) diff --git a/docs/contributing/bot-identities.md b/docs/contributing/bot-identities.md index 6335efa5a2..2095f93572 100644 --- a/docs/contributing/bot-identities.md +++ b/docs/contributing/bot-identities.md @@ -19,3 +19,43 @@ When referencing bot identities in code (e.g., trusted actor lists, dispatch fil **REST vs. GraphQL login format:** the `[bot]` suffix above is the REST/App-slug form. GitHub's GraphQL API omits it — a bot author's `login` field comes back as `fullsend-ai-coder`, not `fullsend-ai-coder[bot]`, with `__typename: "Bot"`. Comparing a GraphQL-sourced login against a literal `"...[bot]"` string never matches (see #5575) — match on `__typename == "Bot"` plus the un-suffixed login instead. **`gh pr view --json` format:** the `gh pr view --json author` CLI command uses a different schema than raw GraphQL — it exposes `.author.is_bot` (boolean) and `.author.login` (with an `app/` prefix, e.g. `app/fullsend-ai-coder`), but does **not** expose `__typename`. When using `gh pr view --json`, check `.author.is_bot == true` plus `.author.login` against the `app/`-prefixed name (see #5536). + +## Per-commit DCO classification + +DCO (Developer Certificate of Origin) eligibility must be classified **per commit**, using the commit author/committer email — never by the fact that a bot-triggered run is operating on the branch. Mixed-author branches (human commits + bot commits) are common on fix-agent PRs. + +**Rules:** + +1. **Bot-authored commits** (committer email matches `+[bot]@users.noreply.github.com`) are exempt from DCO. They must **not** carry a `Signed-off-by` trailer. The Probot DCO app auto-skips them. +2. **Human-authored commits** require valid `Signed-off-by` trailers. These trailers must be **preserved** through any rebase, amend, or history rewrite performed by post-scripts or validation logic. +3. **Never use branch-wide `git filter-branch --msg-filter`** to strip `Signed-off-by` trailers. This destroys valid human attestations on mixed-author branches. See #6688 for the incident this caused. + +**Identifying bot commits in Go code:** + +```go +import "github.com/fullsend-ai/fullsend/internal/forge" + +if forge.IsBotCommitEmail(committerEmail) { + // Bot commit — exempt from DCO, must not have Signed-off-by +} +``` + +**Identifying bot commits in shell (post-scripts):** + +```bash +# Use GIT_BOT_EMAIL (set by the "Resolve bot identity" workflow step) +# for exact match, or the regex pattern for general detection. +committer_email=$(git log -1 --format='%ae' "$sha") + +# Exact match against the resolved bot identity: +if [[ "$committer_email" == "${GIT_BOT_EMAIL}" ]]; then + # Bot commit +fi + +# Pattern match for any GitHub App bot: +if [[ "$committer_email" =~ ^[0-9]+\+.*\[bot\]@users\.noreply\.github\.com$ ]]; then + # Bot commit +fi +``` + +**Post-script guidance:** When a post-script needs to validate or modify DCO trailers, it must iterate over commits individually and classify each by its committer email. Only bot-authored commits should be inspected or modified. Human-authored commits must pass through unmodified. diff --git a/internal/forge/forge.go b/internal/forge/forge.go index 1e1e791d44..0195b58e84 100644 --- a/internal/forge/forge.go +++ b/internal/forge/forge.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "io" + "regexp" "strings" ) @@ -378,6 +379,38 @@ func FormatSignOffTrailer(name, email string) (string, error) { return fmt.Sprintf("Signed-off-by: %s <%s>", name, email), nil } +// botNoreplyRe matches GitHub App bot noreply emails: +// +[bot]@users.noreply.github.com +// +// GitHub generates this address from the App's database ID and slug when +// the App authenticates via an installation token. The Probot DCO app +// auto-skips commits from authors whose email matches this pattern +// (user.type == "Bot"), so bot-authored commits must NOT carry a +// Signed-off-by trailer and human-authored commits on the same branch +// must NOT have their trailers stripped. +// +// See docs/contributing/bot-identities.md for the authoritative identity +// table and DCO classification guidance. +var botNoreplyRe = regexp.MustCompile( + `^\d+\+.+\[bot\]@users\.noreply\.github\.com$`, +) + +// IsBotCommitEmail reports whether email matches the GitHub App bot +// noreply pattern (+[bot]@users.noreply.github.com). +// +// Use this to classify commits by author type for per-commit DCO +// decisions: bot-authored commits are exempt from DCO sign-off and +// must not carry a Signed-off-by trailer; human-authored commits +// require sign-off and their trailers must be preserved. +// +// Post-scripts and validation scripts should use this classification +// (or the equivalent shell pattern) instead of branch-wide operations +// like git filter-branch --msg-filter, which destroy valid human +// trailers on mixed-author branches. See #6688. +func IsBotCommitEmail(email string) bool { + return botNoreplyRe.MatchString(email) +} + // TreeFile represents a file to be committed via the Git Trees API. // Mode controls file permissions: "100644" for regular files, // "100755" for executable files (e.g., shell scripts). diff --git a/internal/forge/signoff_test.go b/internal/forge/signoff_test.go index 7b36532bab..7a5ee795ff 100644 --- a/internal/forge/signoff_test.go +++ b/internal/forge/signoff_test.go @@ -59,3 +59,72 @@ func TestUserIdentity_SignOffTrailer(t *testing.T) { require.NoError(t, err) assert.Equal(t, "Signed-off-by: Test User ", got) } + +func TestIsBotCommitEmail(t *testing.T) { + tests := []struct { + name string + email string + want bool + }{ + { + name: "coder bot noreply", + email: "278716306+fullsend-ai-coder[bot]@users.noreply.github.com", + want: true, + }, + { + name: "review bot noreply", + email: "123456+fullsend-ai-review[bot]@users.noreply.github.com", + want: true, + }, + { + name: "triage bot noreply", + email: "999+fullsend-ai-triage[bot]@users.noreply.github.com", + want: true, + }, + { + name: "renovate bot noreply", + email: "456789+renovate-fullsend[bot]@users.noreply.github.com", + want: true, + }, + { + name: "human noreply", + email: "12345+alice@users.noreply.github.com", + want: false, + }, + { + name: "human email", + email: "alice@example.com", + want: false, + }, + { + name: "human corporate email", + email: "ascerra@redhat.com", + want: false, + }, + { + name: "empty string", + email: "", + want: false, + }, + { + name: "bot suffix without noreply domain", + email: "123+myapp[bot]@example.com", + want: false, + }, + { + name: "noreply domain without bot suffix", + email: "123+myapp@users.noreply.github.com", + want: false, + }, + { + name: "bot suffix without numeric id", + email: "abc+myapp[bot]@users.noreply.github.com", + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, IsBotCommitEmail(tt.email)) + }) + } +} From 9294369de5620b1afcda80c38237f62e8259100b Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Sun, 30 Aug 2026 17:04:07 +0000 Subject: [PATCH 2/4] fix: address review feedback on PR #6780 - Fix shell example in bot-identities.md: change %ae (author email) to %ce (committer email) to match the documented intent and variable name - Add trust-boundary caveat to IsBotCommitEmail doc comment noting it is a CI-internal heuristic and must not be the sole DCO enforcement gate Addresses review feedback on #6780 --- docs/contributing/bot-identities.md | 2 +- internal/forge/forge.go | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/contributing/bot-identities.md b/docs/contributing/bot-identities.md index 2095f93572..fc67863202 100644 --- a/docs/contributing/bot-identities.md +++ b/docs/contributing/bot-identities.md @@ -45,7 +45,7 @@ if forge.IsBotCommitEmail(committerEmail) { ```bash # Use GIT_BOT_EMAIL (set by the "Resolve bot identity" workflow step) # for exact match, or the regex pattern for general detection. -committer_email=$(git log -1 --format='%ae' "$sha") +committer_email=$(git log -1 --format='%ce' "$sha") # Exact match against the resolved bot identity: if [[ "$committer_email" == "${GIT_BOT_EMAIL}" ]]; then diff --git a/internal/forge/forge.go b/internal/forge/forge.go index 0195b58e84..c434413ea8 100644 --- a/internal/forge/forge.go +++ b/internal/forge/forge.go @@ -403,6 +403,11 @@ var botNoreplyRe = regexp.MustCompile( // must not carry a Signed-off-by trailer; human-authored commits // require sign-off and their trailers must be preserved. // +// This is a CI-internal heuristic based on email pattern matching. +// Because committer email is user-controlled, this function must not +// be the sole DCO enforcement gate — the Probot DCO app and GitHub +// branch protection status checks provide independent enforcement. +// // Post-scripts and validation scripts should use this classification // (or the equivalent shell pattern) instead of branch-wide operations // like git filter-branch --msg-filter, which destroy valid human From 32a57dc2f842685ca9c1d575f38d257f9cf4b18a Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Sun, 30 Aug 2026 17:34:13 +0000 Subject: [PATCH 3/4] docs: align shell regex with Go botNoreplyRe pattern Change `.*` to `.+` in the shell bot-email regex example so it requires a non-empty slug, matching the Go `botNoreplyRe` regex in internal/forge/forge.go. Also note: PR title should be changed to refactor(#6688) and "Closes #6688" to "Part of #6688" per review feedback (sandbox policy prevented direct PR metadata edits). Addresses review feedback on #6780 --- docs/contributing/bot-identities.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing/bot-identities.md b/docs/contributing/bot-identities.md index fc67863202..fe534f4b9a 100644 --- a/docs/contributing/bot-identities.md +++ b/docs/contributing/bot-identities.md @@ -53,7 +53,7 @@ if [[ "$committer_email" == "${GIT_BOT_EMAIL}" ]]; then fi # Pattern match for any GitHub App bot: -if [[ "$committer_email" =~ ^[0-9]+\+.*\[bot\]@users\.noreply\.github\.com$ ]]; then +if [[ "$committer_email" =~ ^[0-9]+\+.+\[bot\]@users\.noreply\.github\.com$ ]]; then # Bot commit fi ``` From 5e4c8d75b6dde7585eb069d6264c221cf057217c Mon Sep 17 00:00:00 2001 From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com> Date: Sun, 30 Aug 2026 18:05:48 +0000 Subject: [PATCH 4/4] fix: tighten botNoreplyRe to reject embedded @ in slug Replace `.+` with `[^@]+` in the bot noreply regex pattern in both Go code and shell documentation. The `.+` quantifier matched any character including `@`, allowing malformed inputs like `123+foo@bar[bot]@users.noreply.github.com` to incorrectly match. Added test case for the edge case. Addresses review feedback on #6780 --- docs/contributing/bot-identities.md | 2 +- internal/forge/forge.go | 2 +- internal/forge/signoff_test.go | 5 +++++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/contributing/bot-identities.md b/docs/contributing/bot-identities.md index fe534f4b9a..268cebddd8 100644 --- a/docs/contributing/bot-identities.md +++ b/docs/contributing/bot-identities.md @@ -53,7 +53,7 @@ if [[ "$committer_email" == "${GIT_BOT_EMAIL}" ]]; then fi # Pattern match for any GitHub App bot: -if [[ "$committer_email" =~ ^[0-9]+\+.+\[bot\]@users\.noreply\.github\.com$ ]]; then +if [[ "$committer_email" =~ ^[0-9]+\+[^@]+\[bot\]@users\.noreply\.github\.com$ ]]; then # Bot commit fi ``` diff --git a/internal/forge/forge.go b/internal/forge/forge.go index c434413ea8..d305ce0dfa 100644 --- a/internal/forge/forge.go +++ b/internal/forge/forge.go @@ -392,7 +392,7 @@ func FormatSignOffTrailer(name, email string) (string, error) { // See docs/contributing/bot-identities.md for the authoritative identity // table and DCO classification guidance. var botNoreplyRe = regexp.MustCompile( - `^\d+\+.+\[bot\]@users\.noreply\.github\.com$`, + `^\d+\+[^@]+\[bot\]@users\.noreply\.github\.com$`, ) // IsBotCommitEmail reports whether email matches the GitHub App bot diff --git a/internal/forge/signoff_test.go b/internal/forge/signoff_test.go index 7a5ee795ff..7951d51dfa 100644 --- a/internal/forge/signoff_test.go +++ b/internal/forge/signoff_test.go @@ -121,6 +121,11 @@ func TestIsBotCommitEmail(t *testing.T) { email: "abc+myapp[bot]@users.noreply.github.com", want: false, }, + { + name: "embedded at-sign in slug", + email: "123+foo@bar[bot]@users.noreply.github.com", + want: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {