From 9d82059840941445694f08723b720a6679e52098 Mon Sep 17 00:00:00 2001
From: Gusted <postmaster@gusted.xyz>
Date: Tue, 26 Mar 2024 23:28:10 +0100
Subject: [PATCH] [BUG] Do not allow deletion of internal references

- Currently it's possible to modify remote references such as
`refs/pull/<idx>/head` and `refs/heads/<branch>`.
- Disallow that the pull request reference is deleted, as this should
not be at the control of the user. Doing so would result in
inconsistencies within Forgejo and lead to internal server errors when
trying access the pull request, this action should be reserved for
Forgejo.
- Do this by utilizing the `update` hook, which process each reference
individually and therefore allow to only skip deleting internal
references and still allow other modifications that is being done in
the same push.
- Ref: https://codeberg.org/Codeberg/Community/issues/1517
---
 cmd/hook.go                   | 27 +++++++++++++++++++++++++--
 tests/integration/git_test.go | 16 ++++++++++++++++
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/cmd/hook.go b/cmd/hook.go
index 966e4a57ca..70001f6402 100644
--- a/cmd/hook.go
+++ b/cmd/hook.go
@@ -293,9 +293,32 @@ Forgejo or set your environment appropriately.`, "")
 	return nil
 }
 
+// runHookUpdate process the update hook: https://git-scm.com/docs/githooks#update
 func runHookUpdate(c *cli.Context) error {
-	// Update is empty and is kept only for backwards compatibility
-	return nil
+	// Now if we're an internal don't do anything else
+	if isInternal, _ := strconv.ParseBool(os.Getenv(repo_module.EnvIsInternal)); isInternal {
+		return nil
+	}
+
+	ctx, cancel := installSignals()
+	defer cancel()
+
+	// The last three arguments given to the hook are in order: reference name, old commit ID and new commit ID.
+	args := os.Args[len(os.Args)-3:]
+	refFullName := git.RefName(args[0])
+	newCommitID := args[2]
+
+	// Only process pull references.
+	if !refFullName.IsPull() {
+		return nil
+	}
+
+	// Deletion of the ref means that the new commit ID is only composed of '0'.
+	if strings.ContainsFunc(newCommitID, func(e rune) bool { return e != '0' }) {
+		return nil
+	}
+
+	return fail(ctx, fmt.Sprintf("The deletion of %s is skipped as it's an internal reference.", refFullName), "")
 }
 
 func runHookPostReceive(c *cli.Context) error {
diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go
index e0a8246719..782390002c 100644
--- a/tests/integration/git_test.go
+++ b/tests/integration/git_test.go
@@ -84,6 +84,7 @@ func testGit(t *testing.T, u *url.URL) {
 		mediaTest(t, &httpContext, little, big, littleLFS, bigLFS)
 
 		t.Run("CreateAgitFlowPull", doCreateAgitFlowPull(dstPath, &httpContext, "master", "test/head"))
+		t.Run("InternalReferences", doInternalReferences(&httpContext, dstPath))
 		t.Run("BranchProtectMerge", doBranchProtectPRMerge(&httpContext, dstPath))
 		t.Run("AutoMerge", doAutoPRMerge(&httpContext, dstPath))
 		t.Run("CreatePRAndSetManuallyMerged", doCreatePRAndSetManuallyMerged(httpContext, httpContext, dstPath, "master", "test-manually-merge"))
@@ -125,6 +126,7 @@ func testGit(t *testing.T, u *url.URL) {
 			mediaTest(t, &sshContext, little, big, littleLFS, bigLFS)
 
 			t.Run("CreateAgitFlowPull", doCreateAgitFlowPull(dstPath, &sshContext, "master", "test/head2"))
+			t.Run("InternalReferences", doInternalReferences(&sshContext, dstPath))
 			t.Run("BranchProtectMerge", doBranchProtectPRMerge(&sshContext, dstPath))
 			t.Run("MergeFork", func(t *testing.T) {
 				defer tests.PrintCurrentTest(t)()
@@ -734,6 +736,20 @@ func doAutoPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) {
 	}
 }
 
+func doInternalReferences(ctx *APITestContext, dstPath string) func(t *testing.T) {
+	return func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: ctx.Username, Name: ctx.Reponame})
+		pr1 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{HeadRepoID: repo.ID})
+
+		_, stdErr, gitErr := git.NewCommand(git.DefaultContext, "push", "origin").AddDynamicArguments(fmt.Sprintf(":refs/pull/%d/head", pr1.Index)).RunStdString(&git.RunOpts{Dir: dstPath})
+		assert.Error(t, gitErr)
+		assert.Contains(t, stdErr, fmt.Sprintf("remote: Forgejo: The deletion of refs/pull/%d/head is skipped as it's an internal reference.", pr1.Index))
+		assert.Contains(t, stdErr, fmt.Sprintf("[remote rejected] refs/pull/%d/head (hook declined)", pr1.Index))
+	}
+}
+
 func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, baseBranch, headBranch string) func(t *testing.T) {
 	return func(t *testing.T) {
 		defer tests.PrintCurrentTest(t)()