fix(github_repository_file): produce signed delete commits via GraphQL#3365
Open
LudovicTOURMAN wants to merge 2 commits intointegrations:mainfrom
Open
fix(github_repository_file): produce signed delete commits via GraphQL#3365LudovicTOURMAN wants to merge 2 commits intointegrations:mainfrom
LudovicTOURMAN wants to merge 2 commits intointegrations:mainfrom
Conversation
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.
|
👋 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 |
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #3364
Before the change?
github_repository_filedelete commits go throughDELETE /repos/{owner}/{repo}/contents/{path}. UnlikePUTon 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 toGitHub <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 applycreating or updating a file → signed, accepted.terraform destroy(or any change tofile/branchtriggering 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?
resourceGithubRepositoryFileDeletenow uses the GraphQLcreateCommitOnBranchmutation with a singlefileChanges.deletionsentry. 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 (whencommit_author/commit_emailare left unset).Summary of the code change:
getBranchHeadOidhelper that resolves the current tip of the target branch via GraphQL, used asexpectedHeadOidfor the mutation.DeleteFilecall with the mutation; archived-repo error handling is preserved viahandleArchivedRepoDelete.website/docs/r/repository_file.html.markdownexplaining the signing guarantees on delete vs. create/update.Create and update paths are intentionally left on the REST Contents API:
createCommitOnBranchdoes not accept a customauthor/committer, so routing create/update through it would make the existingcommit_author/commit_emailarguments 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
The existing
TestAccGithubRepositoryFilecases 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?
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 whencommit_author/commit_emailare unset.