Re: git-pull and tag objects

From: Junio C Hamano <junkio@cox.net>
Date: Feb 11, 2007, 05:52 UTC
Message-ID: <7vy7n5gs0y.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <Pine.LNX.4.64.0702100938540.8424@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

Show 24 quoted lines

> But if you have pushed things out (or others could just read your 
> repository directly), then others will have already seen the old tag. In 
> that case you can do one of two things:
> ...
>  - The insane thing.
>
>    You really want to call the new version "X" too, _even_though_ others 
>    have already seen the old one. So just use "git tag -f" again, as if 
>    you hadn't already published the old one.
>
>    HOWEVER!
>
>    Git does *not* (and in my very very strong opinion, MUST NOT!) change 
>    tags behind users back. So if somebody already got the old tag, doing a 
>    "git pull" on your tree shouldn't just make them overwrite the old one. 
>
> And I really think that git does the right thing. If somebody got a 
> release tag from you, you cannot just change the tag for them by updating 
> your own one. I think this is a BIG security issue, in that people MUST be 
> able to trust their tag-names. If I got a particular tag, NO WAY IN HELL 
> must git just replace it for me because you happened to have a newer one!
>
> So if you really want to do the insane thing, you need to just fess up to 
> it, and tell people that you messed up.

Confession time.

Although it is correct that the people who already saw the original tag would not lose the tag object from their repository when you publish a replacement tag, we have _always_ overwritten the refs/tags/$tag to point at the new one, effectively losing the original.

* 0a623e7c (Jul 5, 2005)
In this version "git fetch $repo tag v2.6.13" would have done just

	echo "$head" >"$GIT_DIR/$destination"

without checking if it already existed.

* ae2da406 (Aug 22, 2005)

We started checking if the fetched/followed tag already existed with this version. However, the result of the check was only used to say "$tagname: updating with $new_sha1 from $old_sha1" in the status message.

And after numerous code reorganizations of git-fetch throughout its life, this logic has never been touched. This comment around ll. 170 we currently have:

    case "$1" in
    refs/tags/*)
	# Tags need not be pointing at commits so there
	# is no way to guarantee "fast-forward" anyway.

was introduced with 853a3697 (Aug 20, 2005) and stayed there ever since.

I think it is worth fixing this by tightening the rule as you described, even with this late in the game for 1.5.0. The user either needs to force it, or remove it beforehand.

diff --git a/git-fetch.sh b/git-fetch.sh
index 357cac2..1078016 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -169,14 +169,18 @@ update_local_ref () {
 
     case "$1" in
     refs/tags/*)
-	# Tags need not be pointing at commits so there
-	# is no way to guarantee "fast-forward" anyway.
+	# Tags should never be blindly overwritten without user's
+	# consent.
 	if test -n "$oldshort_"
 	then
 		if now_=$(git show-ref --hash "$1") && test "$now_" = "$2"
 		then
 			[ "$verbose" ] && echo >&2 "* $1: same as $3"
 			[ "$verbose" ] && echo >&2 "  $label_: $newshort_" ||:
+		elif test -z "$force$single_force"
+		then
+			echo >&2 "* $1: refusing to update with $3"
+			false
 		else
 			echo >&2 "* $1: updating with $3"
 			echo >&2 "  $label_: $newshort_"