diff --git a/docs/contributing/bot-identities.md b/docs/contributing/bot-identities.md index 6335efa5a..268cebddd 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='%ce' "$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 1e1e791d4..d305ce0df 100644 --- a/internal/forge/forge.go +++ b/internal/forge/forge.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "io" + "regexp" "strings" ) @@ -378,6 +379,43 @@ 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. +// +// 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 +// 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 7b36532ba..7951d51df 100644 --- a/internal/forge/signoff_test.go +++ b/internal/forge/signoff_test.go @@ -59,3 +59,77 @@ 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, + }, + { + 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) { + assert.Equal(t, tt.want, IsBotCommitEmail(tt.email)) + }) + } +}