From 822af74720a3bfe5691bf0aa3acd5907412f6939 Mon Sep 17 00:00:00 2001 From: Ludovic TOURMAN Date: Wed, 22 Apr 2026 17:37:21 +0200 Subject: [PATCH 1/2] fix(github_repository_file): produce signed delete commits via GraphQL The Contents REST API does not trigger GitHub's server-side web-flow commit signing on DELETE the way it does on PUT, so deleting a github_repository_file produces an unsigned commit. When the target branch is protected by a ruleset that requires signed commits, the delete is rejected or lands as an unsigned commit on the protected branch. Route resourceGithubRepositoryFileDelete through the GraphQL createCommitOnBranch mutation with a single fileChanges.deletions entry. That mutation web-flow-signs every commit it produces, regardless of whether the change is an addition or a deletion, so delete commits are now signed identically to create/update commits made through the REST Contents API with unset commit_author/commit_email. The expected-head oid required by the mutation is fetched via a small GraphQL query against the target branch just before the mutation, so the caller does not have to supply it. Archived-repository errors are still handed to handleArchivedRepoDelete. --- github/resource_github_repository_file.go | 86 ++++++++++++++++++-- website/docs/r/repository_file.html.markdown | 2 + 2 files changed, 80 insertions(+), 8 deletions(-) diff --git a/github/resource_github_repository_file.go b/github/resource_github_repository_file.go index 0d29b8bf63..8e5ad07f67 100644 --- a/github/resource_github_repository_file.go +++ b/github/resource_github_repository_file.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/shurcooL/githubv4" ) func resourceGithubRepositoryFile() *schema.Resource { @@ -403,23 +404,92 @@ func resourceGithubRepositoryFileUpdate(ctx context.Context, d *schema.ResourceD } func resourceGithubRepositoryFileDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { - client := meta.(*Owner).v3client + v4 := meta.(*Owner).v4client owner := meta.(*Owner).name repo := d.Get("repository").(string) file := d.Get("file").(string) + branch := d.Get("branch").(string) - opts := resourceGithubRepositoryFileOptions(d) + ctx = tflog.SetField(ctx, "repository", repo) + ctx = tflog.SetField(ctx, "file", file) + ctx = tflog.SetField(ctx, "owner", owner) + ctx = tflog.SetField(ctx, "branch", branch) - if *opts.Message == fmt.Sprintf("Add %s", file) { - opts.Message = new(fmt.Sprintf("Delete %s", file)) + // The Contents REST endpoint DELETE /repos/{owner}/{repo}/contents/{path} + // does not trigger GitHub's server-side web-flow commit signing, whereas + // PUT on the same endpoint does. Using the GraphQL createCommitOnBranch + // mutation produces a web-flow signed commit for deletions as well, which + // is required by rulesets that enforce signed commits on protected + // branches. + headOid, err := getBranchHeadOid(ctx, v4, owner, repo, branch) + if err != nil { + return diag.FromErr(handleArchivedRepoDelete(err, "repository file", file, owner, repo)) } - branch := d.Get("branch").(string) - opts.Branch = new(branch) + message := fmt.Sprintf("Delete %s", file) + if cm, ok := d.GetOk("commit_message"); ok { + if v := cm.(string); v != "" && v != fmt.Sprintf("Add %s", file) { + message = v + } + } - _, _, err := client.Repositories.DeleteFile(ctx, owner, repo, file, opts) - return diag.FromErr(handleArchivedRepoDelete(err, "repository file", file, owner, repo)) + nameWithOwner := githubv4.String(fmt.Sprintf("%s/%s", owner, repo)) + branchName := githubv4.String(branch) + path := githubv4.String(file) + + input := githubv4.CreateCommitOnBranchInput{ + Branch: githubv4.CommittableBranch{ + RepositoryNameWithOwner: &nameWithOwner, + BranchName: &branchName, + }, + Message: githubv4.CommitMessage{ + Headline: githubv4.String(message), + }, + ExpectedHeadOid: githubv4.GitObjectID(headOid), + FileChanges: &githubv4.FileChanges{ + Deletions: &[]githubv4.FileDeletion{{Path: path}}, + }, + } + + var mutation struct { + CreateCommitOnBranch struct { + Commit struct { + Oid githubv4.String + } + } `graphql:"createCommitOnBranch(input: $input)"` + } + + if err := v4.Mutate(ctx, &mutation, input, nil); err != nil { + return diag.FromErr(handleArchivedRepoDelete(err, "repository file", file, owner, repo)) + } + + return nil +} + +func getBranchHeadOid(ctx context.Context, client *githubv4.Client, owner, repo, branch string) (string, error) { + var query struct { + Repository struct { + Ref struct { + Target struct { + Oid githubv4.String + } + } `graphql:"ref(qualifiedName: $ref)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + variables := map[string]any{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "ref": githubv4.String("refs/heads/" + branch), + } + if err := client.Query(ctx, &query, variables); err != nil { + return "", err + } + oid := string(query.Repository.Ref.Target.Oid) + if oid == "" { + return "", fmt.Errorf("could not resolve HEAD of branch %q on %s/%s", branch, owner, repo) + } + return oid, nil } func autoBranchDiffSuppressFunc(k, _, _ string, d *schema.ResourceData) bool { diff --git a/website/docs/r/repository_file.html.markdown b/website/docs/r/repository_file.html.markdown index 8bd65b16fb..e30f8ae273 100644 --- a/website/docs/r/repository_file.html.markdown +++ b/website/docs/r/repository_file.html.markdown @@ -78,6 +78,8 @@ The following arguments are supported: - `commit_message` - (Optional) The commit message when creating, updating or deleting the managed file. +~> **Note on signed commits:** delete commits are produced through GitHub's GraphQL `createCommitOnBranch` mutation so they are web-flow signed and satisfy rulesets that require signed commits. Create and update commits go through the REST Contents API, which is web-flow signed only when `commit_author` and `commit_email` are left unset. + - `overwrite_on_create` - (Optional) Enable overwriting existing files. If set to `true` it will overwrite an existing file with the same name. If set to `false` it will fail if there is an existing file with the same name. - `autocreate_branch` - (Optional) **Deprecated** Automatically create the branch if it could not be found. Defaults to false. Subsequent reads if the branch is deleted will occur from 'autocreate_branch_source_branch'. Use the `github_branch` resource instead. From a691277c9d43885900734e7bd9e3198f6d5c8d4e Mon Sep 17 00:00:00 2001 From: Ludovic TOURMAN Date: Wed, 22 Apr 2026 17:49:34 +0200 Subject: [PATCH 2/2] test(github_repository_file): regress signed-delete behaviour Add a new subtest under TestAccGithubRepositoryFile that creates a file, then destroys it in a second step by re-applying a config without the resource, and asserts the resulting HEAD commit on the branch is signed (commit.verification.verified == true) and references the deleted path. The old REST-based delete path produced an unsigned commit and would make this test fail; the new GraphQL createCommitOnBranch path web-flow signs the commit and makes it pass. This provides a regression guard against any future switch back to the REST Contents API for deletes. --- .../resource_github_repository_file_test.go | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/github/resource_github_repository_file_test.go b/github/resource_github_repository_file_test.go index 57343b6dd8..06bbba325f 100644 --- a/github/resource_github_repository_file_test.go +++ b/github/resource_github_repository_file_test.go @@ -1,13 +1,16 @@ package github import ( + "context" "fmt" "regexp" "strings" "testing" + "github.com/google/go-github/v85/github" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/terraform" ) func TestAccGithubRepositoryFile(t *testing.T) { @@ -455,4 +458,95 @@ func TestAccGithubRepositoryFile(t *testing.T) { }, }) }) + + t.Run("produces a signed commit when deleting a managed file", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + repoName := fmt.Sprintf("%srepo-file-signed-del-%s", testResourcePrefix, randomID) + filename := "signed-delete-test.md" + + withFile := fmt.Sprintf(` + resource "github_repository" "test" { + name = "%s" + auto_init = true + } + + resource "github_repository_file" "test" { + repository = github_repository.test.name + branch = "main" + file = "%s" + content = "signed delete test\n" + } + `, repoName, filename) + + withoutFile := fmt.Sprintf(` + resource "github_repository" "test" { + name = "%s" + auto_init = true + } + `, repoName) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnauthenticated(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: withFile, + Check: resource.TestCheckResourceAttr( + "github_repository_file.test", "file", filename, + ), + }, + { + Config: withoutFile, + Check: testAccCheckLatestCommitIsSigned(repoName, "main", filename), + }, + }, + }) + }) +} + +// testAccCheckLatestCommitIsSigned asserts that the HEAD of the given branch is +// a signed commit (commit.verification.verified == true) and that its message +// references the expected file path. It is used to guard against regressing the +// delete path off the GraphQL createCommitOnBranch mutation, since the REST +// Contents API produces unsigned commits on DELETE. +func testAccCheckLatestCommitIsSigned(repoName, branch, expectedPathInMessage string) resource.TestCheckFunc { + return func(_ *terraform.State) error { + meta, err := getTestMeta() + if err != nil { + return err + } + + ctx := context.Background() + commits, _, err := meta.v3client.Repositories.ListCommits(ctx, meta.name, repoName, &github.CommitsListOptions{ + SHA: branch, + ListOptions: github.ListOptions{ + PerPage: 1, + }, + }) + if err != nil { + return fmt.Errorf("listing commits on %s/%s@%s: %w", meta.name, repoName, branch, err) + } + if len(commits) == 0 { + return fmt.Errorf("no commits found on %s/%s@%s", meta.name, repoName, branch) + } + + head := commits[0] + msg := head.GetCommit().GetMessage() + if !strings.Contains(msg, expectedPathInMessage) { + return fmt.Errorf( + "expected HEAD commit on %s/%s@%s to reference %q, got message %q (sha %s)", + meta.name, repoName, branch, expectedPathInMessage, msg, head.GetSHA(), + ) + } + + verification := head.GetCommit().GetVerification() + if !verification.GetVerified() { + return fmt.Errorf( + "expected HEAD commit on %s/%s@%s (sha %s, message %q) to be signed, got verified=false reason=%q", + meta.name, repoName, branch, head.GetSHA(), msg, verification.GetReason(), + ) + } + + return nil + } }