diff --git a/.github/workflows/orchestrate.yaml b/.github/workflows/orchestrate.yaml index d76c1c56..35cf4892 100644 --- a/.github/workflows/orchestrate.yaml +++ b/.github/workflows/orchestrate.yaml @@ -214,7 +214,11 @@ jobs: API_ARGS=("repos/${{ github.repository }}/contents/$MANIFEST_FILE" -X PUT -f "message=chore: update state [skip ci]" -f "content=$CONTENT_B64" - -f "branch=$BRANCH") + -f "branch=$BRANCH" + -f "author[name]=github-actions[bot]" + -f "author[email]=github-actions[bot]@users.noreply.github.com" + -f "committer[name]=github-actions[bot]" + -f "committer[email]=github-actions[bot]@users.noreply.github.com") if [[ -n "$CURRENT_SHA" ]]; then API_ARGS+=(-f "sha=$CURRENT_SHA") fi diff --git a/internal/generate/generator.go b/internal/generate/generator.go index 7f85079b..fc06c5af 100644 --- a/internal/generate/generator.go +++ b/internal/generate/generator.go @@ -1637,6 +1637,8 @@ func (g *Generator) writeManifestUpdateStep(sb *strings.Builder, sorted []string commitMessage: commitMessage, noChangeLabel: "No state changes", successLabel: "Pushed state", + authorName: g.config.GetGitUserName(), + authorEmail: g.config.GetGitUserEmail(), }) } diff --git a/internal/generate/release.go b/internal/generate/release.go index e87d440c..0041f498 100644 --- a/internal/generate/release.go +++ b/internal/generate/release.go @@ -424,6 +424,8 @@ func (g *ReleaseGenerator) writeFinalizeJob(sb *strings.Builder) { commitMessage: "chore: update latest_release state\n\nVersion: $SEMVER_TAG", noChangeLabel: "No latest_release state changes", successLabel: "Pushed latest_release state", + authorName: g.config.GetGitUserName(), + authorEmail: g.config.GetGitUserEmail(), }) // Summary diff --git a/internal/generate/state_write.go b/internal/generate/state_write.go index 758d9cad..ad718c70 100644 --- a/internal/generate/state_write.go +++ b/internal/generate/state_write.go @@ -28,8 +28,24 @@ type stateWriteParams struct { // A token able to bypass branch protection lets the write land on a protected // trunk and produces a verified, signed commit. successLabel string + // authorName and authorEmail are the identity stamped as both the author and + // the committer of the Contents API state commit. Without them the API + // attributes the commit to the token owner; callers populate them from the + // manifest git config (GetGitUserName/GetGitUserEmail) so the commit is the + // automation bot. Empty fields fall back to the github-actions[bot] default. + authorName string + authorEmail string } +// defaultStateAuthorName and defaultStateAuthorEmail are the identity stamped on +// a Contents API state commit when a caller supplies no override. They match the +// git-config identity the act/gitea git-commit path uses, so both write paths +// attribute the commit to the automation bot rather than the token owner. +const ( + defaultStateAuthorName = "github-actions[bot]" + defaultStateAuthorEmail = "github-actions[bot]@users.noreply.github.com" +) + // writeStateCommitPush emits the dual-path state-write logic into an already-open // "run: |" block at the given indent. The caller must have already set the // MANIFEST_FILE and BRANCH shell variables and defined the apply function named @@ -55,6 +71,18 @@ func writeStateCommitPush(sb *strings.Builder, indent string, params stateWriteP fmt.Fprintf(sb, indent+format+"\n", args...) } + // Resolve the commit identity so an absent override never leaves the API write + // to default to the token owner. Callers pass the manifest git config identity; + // an empty field falls back to the automation bot. + authorName := params.authorName + if authorName == "" { + authorName = defaultStateAuthorName + } + authorEmail := params.authorEmail + if authorEmail == "" { + authorEmail = defaultStateAuthorEmail + } + w("if [[ \"$GITHUB_SERVER_URL\" != \"https://github.com\" ]]; then") // gitea/act path: keep the existing git fetch/reset/reapply/commit/push loop. w(" # act/gitea e2e: no GitHub API, and the trunk is neither protected nor") @@ -99,7 +127,13 @@ func writeStateCommitPush(sb *strings.Builder, indent string, params stateWriteP w(" API_ARGS=(\"repos/${{ github.repository }}/contents/$MANIFEST_FILE\" -X PUT") w(" -f \"message=%s\"", shellSingleLineMessage(params.commitMessage)) w(" -f \"content=$CONTENT_B64\"") - w(" -f \"branch=$BRANCH\")") + w(" -f \"branch=$BRANCH\"") + // Stamp author and committer so the API attributes the commit to the bot + // identity rather than the token owner GitHub would otherwise use. + w(" -f \"author[name]=%s\"", authorName) + w(" -f \"author[email]=%s\"", authorEmail) + w(" -f \"committer[name]=%s\"", authorName) + w(" -f \"committer[email]=%s\")", authorEmail) w(" if [[ -n \"$CURRENT_SHA\" ]]; then") w(" API_ARGS+=(-f \"sha=$CURRENT_SHA\")") w(" fi") diff --git a/internal/generate/state_write_test.go b/internal/generate/state_write_test.go index 7d9cc010..30c33b3e 100644 --- a/internal/generate/state_write_test.go +++ b/internal/generate/state_write_test.go @@ -132,6 +132,86 @@ func TestPromoteFinalizeStateTokenAuth(t *testing.T) { "Finalize Promotion must auth the API state write with state_token") } +// assertAPIAuthorStamp asserts the Contents API state-write path stamps both the +// author and the committer with the given identity, so an API-created state +// commit is attributed to the bot rather than the token owner GitHub would +// otherwise default to. +func assertAPIAuthorStamp(t *testing.T, content, name, email string) { + t.Helper() + for _, want := range []string{ + `-f "author[name]=` + name + `"`, + `-f "author[email]=` + email + `"`, + `-f "committer[name]=` + name + `"`, + `-f "committer[email]=` + email + `"`, + } { + assert.Contains(t, content, want, + "Contents API state write must stamp the bot identity on author and committer") + } +} + +// TestOrchestrateFinalizeStampsBotAuthor verifies the orchestrate state-write API +// path attributes the commit to the github-actions[bot] default rather than the +// token owner. +func TestOrchestrateFinalizeStampsBotAuthor(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".github/workflows"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, ".github/workflows/build.yaml"), []byte("on:\n workflow_call:\n"), 0644)) + + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: []string{"dev"}, + Builds: []config.BuildConfig{ + {Name: "app", Workflow: ".github/workflows/build.yaml", Triggers: []string{"src/**"}}, + }, + } + + content, err := NewGenerator(cfg, tmpDir).Generate() + require.NoError(t, err) + + assertAPIAuthorStamp(t, content, "github-actions[bot]", "github-actions[bot]@users.noreply.github.com") +} + +// TestReleaseFinalizeStampsBotAuthor verifies the release latest_release state +// write attributes the API commit to the bot default. +func TestReleaseFinalizeStampsBotAuthor(t *testing.T) { + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: []string{"prod"}, + } + + content, err := NewReleaseGenerator(cfg, "").Generate() + require.NoError(t, err) + + assertAPIAuthorStamp(t, content, "github-actions[bot]", "github-actions[bot]@users.noreply.github.com") +} + +// TestStateWriteHonorsCustomGitIdentity verifies a manifest git config override +// flows into the Contents API author and committer fields, so operators can +// attribute automated state commits to a custom identity. +func TestStateWriteHonorsCustomGitIdentity(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".github/workflows"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, ".github/workflows/build.yaml"), []byte("on:\n workflow_call:\n"), 0644)) + + cfg := &config.TrunkConfig{ + TrunkBranch: "main", + Environments: []string{"dev"}, + Git: &config.GitConfig{ + Mode: config.GitModeCustom, + UserName: "release-bot", + UserEmail: "release-bot@example.com", + }, + Builds: []config.BuildConfig{ + {Name: "app", Workflow: ".github/workflows/build.yaml", Triggers: []string{"src/**"}}, + }, + } + + content, err := NewGenerator(cfg, tmpDir).Generate() + require.NoError(t, err) + + assertAPIAuthorStamp(t, content, "release-bot", "release-bot@example.com") +} + // TestStateWriteNoEmDash guards the hard project rule that generated output // contains no em dashes. func TestStateWriteNoEmDash(t *testing.T) { diff --git a/internal/rollback/command_subcommands.go b/internal/rollback/command_subcommands.go index 2a26618f..5df0d796 100644 --- a/internal/rollback/command_subcommands.go +++ b/internal/rollback/command_subcommands.go @@ -12,6 +12,7 @@ import ( "github.com/stablekernel/cascade/internal/config" "github.com/stablekernel/cascade/internal/ghaoutput" + "github.com/stablekernel/cascade/internal/statewrite" ) // newPreflightCommand creates the `cascade rollback preflight` subcommand. It @@ -213,7 +214,7 @@ func runFinalize(opts finalizeOptions) error { } if opts.commitPush { - if err := commitAndPush(rb.ConfigPath(), plan.Environment); err != nil { + if err := commitAndPush(rb.ConfigPath(), plan.Environment, rb.GitIdentity()); err != nil { return fmt.Errorf("failed to commit and push: %w", err) } fmt.Printf("State updated and committed for %s\n", plan.Environment) @@ -290,7 +291,7 @@ func readDeployResultsFromEnv(deployNames []string) map[string]string { // update a protected trunk. In the act/gitea environment there is no GitHub API, // so the change is committed and pushed with plain git. The environment is // detected exactly as the promote finalize path does, by GITHUB_SERVER_URL. -func commitAndPush(path, env string) error { +func commitAndPush(path, env string, author statewrite.Identity) error { status, err := exec.Command("git", "status", "--porcelain", path).Output() if err != nil { return fmt.Errorf("git status failed: %w", err) @@ -302,9 +303,9 @@ func commitAndPush(path, env string) error { message := fmt.Sprintf("chore: update state after rollback of %s [skip ci]", env) if isRealGitHub() { - return writeStateViaAPI(path, message) + return writeStateViaAPI(path, message, author) } - return commitAndPushGit(path, message) + return commitAndPushGit(path, message, author) } // isRealGitHub reports whether the workflow runs on github.com rather than an @@ -317,7 +318,7 @@ func isRealGitHub() bool { // writeStateViaAPI writes the manifest to the trunk branch through the GitHub // Contents REST API using the gh CLI, producing a signed commit that can update // a protected branch when the token is bypass-capable. -func writeStateViaAPI(path, message string) error { +func writeStateViaAPI(path, message string, author statewrite.Identity) error { repo := os.Getenv("GITHUB_REPOSITORY") if repo == "" { return fmt.Errorf("GITHUB_REPOSITORY is not set; cannot write state via API") @@ -335,29 +336,45 @@ func writeStateViaAPI(path, message string) error { shaOut, _ := exec.Command("gh", "api", fmt.Sprintf("%s?ref=%s", apiPath, branch), "--jq", ".sha").Output() currentSHA := strings.TrimSpace(string(shaOut)) + args := buildStatePutArgs(apiPath, branch, currentSHA, message, contentB64, author) + + if out, err := exec.Command("gh", args...).CombinedOutput(); err != nil { + return fmt.Errorf("state write via API failed: %s: %w", strings.TrimSpace(string(out)), err) + } + return nil +} + +// buildStatePutArgs assembles the gh CLI arguments for the Contents API PUT that +// writes the manifest. It stamps both author and committer with the resolved +// identity so the API attributes the commit to the bot rather than the token +// owner GitHub would otherwise use when those fields are absent. An empty +// currentSHA creates the file rather than guarding an update. +func buildStatePutArgs(apiPath, branch, currentSHA, message, contentB64 string, author statewrite.Identity) []string { + id := author.OrDefault() args := []string{ "api", apiPath, "-X", "PUT", "-f", "message=" + message, "-f", "content=" + contentB64, "-f", "branch=" + branch, + "-f", "author[name]=" + id.Name, + "-f", "author[email]=" + id.Email, + "-f", "committer[name]=" + id.Name, + "-f", "committer[email]=" + id.Email, } if currentSHA != "" { args = append(args, "-f", "sha="+currentSHA) } - - if out, err := exec.Command("gh", args...).CombinedOutput(); err != nil { - return fmt.Errorf("state write via API failed: %s: %w", strings.TrimSpace(string(out)), err) - } - return nil + return args } // commitAndPushGit commits the manifest and pushes with plain git, used in the // act/gitea environment which enforces neither branch protection nor signatures. -func commitAndPushGit(path, message string) error { - if err := exec.Command("git", "config", "user.name", "github-actions[bot]").Run(); err != nil { +func commitAndPushGit(path, message string, author statewrite.Identity) error { + id := author.OrDefault() + if err := exec.Command("git", "config", "user.name", id.Name).Run(); err != nil { return fmt.Errorf("git config user.name failed: %w", err) } - if err := exec.Command("git", "config", "user.email", "github-actions[bot]@users.noreply.github.com").Run(); err != nil { + if err := exec.Command("git", "config", "user.email", id.Email).Run(); err != nil { return fmt.Errorf("git config user.email failed: %w", err) } if err := exec.Command("git", "add", path).Run(); err != nil { diff --git a/internal/rollback/command_subcommands_test.go b/internal/rollback/command_subcommands_test.go index 4e1ea679..638746e7 100644 --- a/internal/rollback/command_subcommands_test.go +++ b/internal/rollback/command_subcommands_test.go @@ -8,6 +8,7 @@ import ( "github.com/stablekernel/cascade/internal/config" "github.com/stablekernel/cascade/internal/promote" + "github.com/stablekernel/cascade/internal/statewrite" ) // ringManifest writes a manifest whose prod env carries a deploy-history ring @@ -505,3 +506,45 @@ func TestRollbackPreflight_GHAOutput_EmitsTargetSource_DefaultPreviousRing(t *te t.Errorf("can_proceed not true\n%s", got) } } + +// TestBuildStatePutArgs_StampsBotAuthor verifies the rollback Contents API state +// write attributes the commit to the github-actions[bot] default, not the token +// owner GitHub would otherwise stamp when author and committer are absent. +func TestBuildStatePutArgs_StampsBotAuthor(t *testing.T) { + args := buildStatePutArgs("repos/acme/widgets/contents/m.yaml", "main", "deadbeef", "chore: update state after rollback of prod [skip ci]", "Y29udGVudA==", statewrite.Identity{}) + + joined := strings.Join(args, "\x00") + for _, want := range []string{ + "author[name]=github-actions[bot]", + "author[email]=github-actions[bot]@users.noreply.github.com", + "committer[name]=github-actions[bot]", + "committer[email]=github-actions[bot]@users.noreply.github.com", + "sha=deadbeef", + } { + if !strings.Contains(joined, want) { + t.Errorf("buildStatePutArgs missing %q in args %v", want, args) + } + } +} + +// TestBuildStatePutArgs_HonorsCustomIdentity verifies a manifest git override +// flows into the rollback API author and committer fields. +func TestBuildStatePutArgs_HonorsCustomIdentity(t *testing.T) { + id := statewrite.Identity{Name: "release-bot", Email: "release-bot@example.com"} + args := buildStatePutArgs("repos/acme/widgets/contents/m.yaml", "main", "", "msg", "Y29udGVudA==", id) + + joined := strings.Join(args, "\x00") + for _, want := range []string{ + "author[name]=release-bot", + "author[email]=release-bot@example.com", + "committer[name]=release-bot", + "committer[email]=release-bot@example.com", + } { + if !strings.Contains(joined, want) { + t.Errorf("buildStatePutArgs missing %q in args %v", want, args) + } + } + if strings.Contains(joined, "sha=") { + t.Errorf("empty sha must not add a sha arg: %v", args) + } +} diff --git a/internal/rollback/rollback.go b/internal/rollback/rollback.go index 45f3c6c4..eb93df19 100644 --- a/internal/rollback/rollback.go +++ b/internal/rollback/rollback.go @@ -17,6 +17,7 @@ import ( "github.com/stablekernel/cascade/internal/config" "github.com/stablekernel/cascade/internal/promote" + "github.com/stablekernel/cascade/internal/statewrite" "gopkg.in/yaml.v3" ) @@ -126,6 +127,20 @@ func (r *Rollbacker) ConfigPath() string { return r.configPath } +// GitIdentity returns the commit identity for the post-rollback state write, +// taken from the manifest git config so an automated rollback commit is +// attributed to the configured bot rather than the token owner. An absent or +// empty git config resolves to the github-actions[bot] default. +func (r *Rollbacker) GitIdentity() statewrite.Identity { + if r.cicdFile == nil || r.cicdFile.Config == nil { + return statewrite.Identity{} + } + return statewrite.Identity{ + Name: r.cicdFile.Config.GetGitUserName(), + Email: r.cicdFile.Config.GetGitUserEmail(), + } +} + // DeployNames returns the names of the deploys declared in the manifest, in // declaration order. The finalize subcommand uses it to gate the state write on // each deploy job's reported result. It returns nil when no deploys are diff --git a/internal/statewrite/apiwrite.go b/internal/statewrite/apiwrite.go index c6b17ff7..af6e75bb 100644 --- a/internal/statewrite/apiwrite.go +++ b/internal/statewrite/apiwrite.go @@ -46,10 +46,12 @@ type Identity struct { Email string } -// orDefault returns the identity with any empty field filled from the bot +// OrDefault returns the identity with any empty field filled from the bot // default, so a commit is always attributed to a concrete identity and behavior -// is never worse than before this attribution was threaded through. -func (id Identity) orDefault() Identity { +// is never worse than before this attribution was threaded through. It is +// exported so other state writers that build their own Contents API request can +// resolve the same identity without duplicating the bot defaults. +func (id Identity) OrDefault() Identity { if id.Name == "" { id.Name = defaultBotName } @@ -174,7 +176,7 @@ func CommitWithRetry(opts Options) error { if sleep == nil { sleep = time.Sleep } - author := opts.Author.orDefault() + author := opts.Author.OrDefault() var lastErr error for attempt := 1; attempt <= maxAttempts; attempt++ { diff --git a/internal/statewrite/ghclient.go b/internal/statewrite/ghclient.go index 56ed5572..9ea32ff0 100644 --- a/internal/statewrite/ghclient.go +++ b/internal/statewrite/ghclient.go @@ -48,7 +48,7 @@ func (ghContents) GetContent(repo, path, ref string) ([]byte, string, error) { // the token owner, and classifies a 409 optimistic-lock failure as a // ConflictError so the retry loop recognizes it. func (ghContents) PutContent(repo, path, ref, sha, message string, content []byte, author Identity) error { - author = author.orDefault() + author = author.OrDefault() b64 := base64.StdEncoding.EncodeToString(content) args := []string{ "api", fmt.Sprintf("repos/%s/contents/%s", repo, path), "-X", "PUT",