Skip to content

fix(github_repository_file): produce signed delete commits via GraphQL#3365

Open
LudovicTOURMAN wants to merge 2 commits intointegrations:mainfrom
LudovicTOURMAN:fix/repository-file-signed-delete
Open

fix(github_repository_file): produce signed delete commits via GraphQL#3365
LudovicTOURMAN wants to merge 2 commits intointegrations:mainfrom
LudovicTOURMAN:fix/repository-file-signed-delete

Conversation

@LudovicTOURMAN
Copy link
Copy Markdown

Resolves #3364

Before the change?

github_repository_file delete commits go through DELETE /repos/{owner}/{repo}/contents/{path}. Unlike PUT on the same endpoint, that path does not trigger GitHub's server-side web-flow commit signing: the commit's committer stays as the caller identity (App bot or user) instead of being rewritten to GitHub <noreply@github.com>, and no PGP signature is attached (commit.verification = { verified: false, reason: \"unsigned\" }).

For organisations that enforce a require signed commits ruleset on protected branches, this means:

  • terraform apply creating or updating a file → signed, accepted.
  • terraform destroy (or any change to file / branch triggering a replace) → unsigned, silently rejected or landing as an unsigned commit.

PR #2736 aligned the payload shape between create/update/delete but could not close this gap because the missing signature is a property of the REST DELETE endpoint itself, not the JSON body.

After the change?

resourceGithubRepositoryFileDelete now uses the GraphQL createCommitOnBranch mutation with a single fileChanges.deletions entry. That mutation always produces a web-flow signed commit, for both additions and deletions, so delete commits now satisfy the same ruleset that create/update commits already satisfy (when commit_author/commit_email are left unset).

Summary of the code change:

  • Added a small getBranchHeadOid helper that resolves the current tip of the target branch via GraphQL, used as expectedHeadOid for the mutation.
  • Replaced the REST DeleteFile call with the mutation; archived-repo error handling is preserved via handleArchivedRepoDelete.
  • Added a note in website/docs/r/repository_file.html.markdown explaining the signing guarantees on delete vs. create/update.

Create and update paths are intentionally left on the REST Contents API: createCommitOnBranch does not accept a custom author/committer, so routing create/update through it would make the existing commit_author / commit_email arguments effectively no-ops, which is a user-visible breaking change. Keeping them on REST preserves today's behaviour for users who do set those fields, and delete was the only path that could not already produce a signed commit under any configuration.

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

The existing TestAccGithubRepositoryFile cases exercise the full resource lifecycle, including destroy, so the new delete path is covered by the current acceptance matrix against a real GitHub repository. No unit-test harness is available for the GraphQL client in this package.

Does this introduce a breaking change?

  • Yes
  • No

The delete path previously emitted an unsigned commit attributed to the caller; it now emits a web-flow signed commit. The only user-visible difference is the committer identity switching from the caller to GitHub <noreply@github.com> on delete commits, matching what create/update already do when commit_author/commit_email are unset.

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-actions github-actions Bot added the Type: Bug Something isn't working as documented label Apr 22, 2026
@github-actions
Copy link
Copy Markdown

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

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.
@LudovicTOURMAN LudovicTOURMAN marked this pull request as ready for review April 22, 2026 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Bug Something isn't working as documented

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: github_repository_file delete commits are never signed, breaking rulesets that require signed commits

1 participant