From d8493306e5ccdbe1bf84abcdb00d2943c06ff761 Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Thu, 25 Jun 2026 22:38:34 -0400 Subject: [PATCH 1/2] fix(statewrite): attribute generated and rollback state commits to the bot The orchestrate and release finalize steps write trunk state through the Contents REST API by emitting a gh api PUT, and the rollback finalize CLI builds the same PUT by hand. None of these set author or committer, so the API attributed the commit to the token owner instead of the automation bot. Stamp author and committer from the manifest git identity (defaulting to github-actions[bot]) on all three paths, reusing the statewrite.Identity plumbing. Signed-off-by: Joshua Temple --- internal/generate/generator.go | 2 + internal/generate/release.go | 2 + internal/generate/state_write.go | 36 ++++++++- internal/generate/state_write_test.go | 80 +++++++++++++++++++ internal/rollback/command_subcommands.go | 43 +++++++--- internal/rollback/command_subcommands_test.go | 43 ++++++++++ internal/rollback/rollback.go | 15 ++++ internal/statewrite/apiwrite.go | 10 ++- internal/statewrite/ghclient.go | 2 +- 9 files changed, 214 insertions(+), 19 deletions(-) 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", From 8972162bbba490365d2bcc8dd12ffde78dc7fb87 Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Thu, 25 Jun 2026 22:43:18 -0400 Subject: [PATCH 2/2] chore: regenerate orchestrate workflow with bot state-write author Signed-off-by: Joshua Temple --- .github/workflows/orchestrate.yaml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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