On undoing a forced push

On undoing a forced push

[Duy Nguyen]

>From a thread on Hacker News. It seems that if a user does not have
access to the remote's reflog and accidentally forces a push to a ref,
how does he recover it? In order to force push again to revert it
back, he would need to know the remote's old SHA-1. Local reflog does
not help because remote refs are not updated during a push.

This patch prints the latest SHA-1 before the forced push in full. He
then can do

    git push <remote> +<old-sha1>:<ref>

He does not even need to have the objects that <old-sha1> refers
to. We could simply push an empty pack and the the remote will happily
accept the force, assuming garbage collection has not happened. But
that's another and a little more complex patch.

Is there any other way to undo a forced push?

-- 8< --
diff --git a/transport.c b/transport.c
index f080e93..6bd6a64 100644
--- a/transport.c
+++ b/transport.c
@@ -657,16 +657,17 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
 			"[new branch]"),
 			ref, ref->peer_ref, NULL, porcelain);
 	else {
-		char quickref[84];
+		char quickref[104];
 		char type;
 		const char *msg;
 
-		strcpy(quickref, status_abbrev(ref->old_sha1));
 		if (ref->forced_update) {
+			strcpy(quickref, sha1_to_hex(ref->old_sha1));
 			strcat(quickref, "...");
 			type = '+';
 			msg = "forced update";
 		} else {
+			strcpy(quickref, status_abbrev(ref->old_sha1));
 			strcat(quickref, "..");
 			type = ' ';
 			msg = NULL;
-- 8< --

Re: On undoing a forced push

[Matthieu Moy]

Duy Nguyen <pclouds@gmail.com> writes:

> From a thread on Hacker News. It seems that if a user does not have
> access to the remote's reflog and accidentally forces a push to a ref,
> how does he recover it? In order to force push again to revert it
> back, he would need to know the remote's old SHA-1. Local reflog does
> not help because remote refs are not updated during a push.

More precisely, the remote-tracking ref is updated, and so is its
reflog, but the reflog entry usually does not help, because it documents
the old and new sha1 of the remote-tracking ref, not of the remote ref
itself. Typically, if a coworker pushed some code that I did not pull,
and I force-push to the same branch, my reflog won't have the sha1 of my
coworker's code.

> This patch prints the latest SHA-1 before the forced push in full.

Sounds like a good idea to me.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: On undoing a forced push

[Sitaram Chamarty]

On 06/09/2015 05:42 PM, Duy Nguyen wrote:
> From a thread on Hacker News. It seems that if a user does not have
> access to the remote's reflog and accidentally forces a push to a ref,
> how does he recover it? In order to force push again to revert it
> back, he would need to know the remote's old SHA-1. Local reflog does
> not help because remote refs are not updated during a push.
> 
> This patch prints the latest SHA-1 before the forced push in full. He
> then can do
> 
>     git push <remote> +<old-sha1>:<ref>
> 
> He does not even need to have the objects that <old-sha1> refers
> to. We could simply push an empty pack and the the remote will happily
> accept the force, assuming garbage collection has not happened. But
> that's another and a little more complex patch.

If I am not mistaken, we actively prevent people from downloading an
unreferenced SHA (such as would happen if you overwrote refs that
contained sensitive information like passwords).

Wouldn't allowing the kind of push you just described, require negating
that protection?

Re: On undoing a forced push

[Jeff King]

On Tue, Jun 09, 2015 at 07:36:20PM +0530, Sitaram Chamarty wrote:

> > This patch prints the latest SHA-1 before the forced push in full. He
> > then can do
> > 
> >     git push <remote> +<old-sha1>:<ref>
> > 
> > He does not even need to have the objects that <old-sha1> refers
> > to. We could simply push an empty pack and the the remote will happily
> > accept the force, assuming garbage collection has not happened. But
> > that's another and a little more complex patch.
> 
> If I am not mistaken, we actively prevent people from downloading an
> unreferenced SHA (such as would happen if you overwrote refs that
> contained sensitive information like passwords).
> 
> Wouldn't allowing the kind of push you just described, require negating
> that protection?

No, this has always worked. If you have write access to a repository,
you can fetch anything from it with this trick. Even if we blocked this,
there are other ways to leak information. For instance, I can push up
objects that are "similar" to the target object, claim to have the
target object, and then hope git will make a delta between my similar
object and the target. Iterate on the "similar" object and you can
eventually figure out what is in the target object.

This is one of the reasons we do not share objects between public and
private forks at GitHub.

-Peff

Re: On undoing a forced push

[Sitaram Chamarty]

On 06/09/2015 07:55 PM, Jeff King wrote:
> On Tue, Jun 09, 2015 at 07:36:20PM +0530, Sitaram Chamarty wrote:
> 
>>> This patch prints the latest SHA-1 before the forced push in full. He
>>> then can do
>>>
>>>     git push <remote> +<old-sha1>:<ref>
>>>
>>> He does not even need to have the objects that <old-sha1> refers
>>> to. We could simply push an empty pack and the the remote will happily
>>> accept the force, assuming garbage collection has not happened. But
>>> that's another and a little more complex patch.
>>
>> If I am not mistaken, we actively prevent people from downloading an
>> unreferenced SHA (such as would happen if you overwrote refs that
>> contained sensitive information like passwords).
>>
>> Wouldn't allowing the kind of push you just described, require negating
>> that protection?
> 
> No, this has always worked. If you have write access to a repository,
> you can fetch anything from it with this trick. Even if we blocked this,
> there are other ways to leak information. For instance, I can push up
> objects that are "similar" to the target object, claim to have the
> target object, and then hope git will make a delta between my similar
> object and the target. Iterate on the "similar" object and you can
> eventually figure out what is in the target object.

aah ok; I must have mis-remembered something.  Thanks!

Re: On undoing a forced push

[Johannes Schindelin]

Hi,

On 2015-06-09 16:06, Sitaram Chamarty wrote:
> On 06/09/2015 05:42 PM, Duy Nguyen wrote:
>> From a thread on Hacker News. It seems that if a user does not have
>> access to the remote's reflog and accidentally forces a push to a ref,
>> how does he recover it? In order to force push again to revert it
>> back, he would need to know the remote's old SHA-1. Local reflog does
>> not help because remote refs are not updated during a push.
>>
>> This patch prints the latest SHA-1 before the forced push in full. He
>> then can do
>>
>>     git push <remote> +<old-sha1>:<ref>
>>
>> He does not even need to have the objects that <old-sha1> refers
>> to. We could simply push an empty pack and the the remote will happily
>> accept the force, assuming garbage collection has not happened. But
>> that's another and a little more complex patch.
> 
> If I am not mistaken, we actively prevent people from downloading an
> unreferenced SHA (such as would happen if you overwrote refs that
> contained sensitive information like passwords).
> 
> Wouldn't allowing the kind of push you just described, require negating
> that protection?

I believe that to be the case.

Sorry to chime in so late in the discussion, but I think that the `--force-with-lease` option is what you are looking for. It allows you to force-push *but only* if the forced push would overwrite the ref we expect, i.e. (simplified, but you get the idea) `git push --force-with-lease <remote> <ref>` will *only* succeed if the remote's <ref> agrees with the local `refs/remotes/<remote>/<ref>`.

If you use `--force-with-lease`, you simply cannot force-forget anything on the remote side that you cannot undo (because you have everything locally you need to undo it).

Ciao,
Johannes

Re: On undoing a forced push

[Stefan Beller]

On Tue, Jun 9, 2015 at 9:29 AM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> Hi,
>
> On 2015-06-09 16:06, Sitaram Chamarty wrote:
>> On 06/09/2015 05:42 PM, Duy Nguyen wrote:
>>> From a thread on Hacker News. It seems that if a user does not have
>>> access to the remote's reflog and accidentally forces a push to a ref,
>>> how does he recover it? In order to force push again to revert it
>>> back, he would need to know the remote's old SHA-1. Local reflog does
>>> not help because remote refs are not updated during a push.
>>>
>>> This patch prints the latest SHA-1 before the forced push in full. He
>>> then can do
>>>
>>>     git push <remote> +<old-sha1>:<ref>
>>>
>>> He does not even need to have the objects that <old-sha1> refers
>>> to. We could simply push an empty pack and the the remote will happily
>>> accept the force, assuming garbage collection has not happened. But
>>> that's another and a little more complex patch.
>>
>> If I am not mistaken, we actively prevent people from downloading an
>> unreferenced SHA (such as would happen if you overwrote refs that
>> contained sensitive information like passwords).
>>
>> Wouldn't allowing the kind of push you just described, require negating
>> that protection?
>
> I believe that to be the case.
>
> Sorry to chime in so late in the discussion, but I think that the `--force-with-lease` option is what you are looking for. It allows you to force-push *but only* if the forced push would overwrite the ref we expect, i.e. (simplified, but you get the idea) `git push --force-with-lease <remote> <ref>` will *only* succeed if the remote's <ref> agrees with the local `refs/remotes/<remote>/<ref>`.

Yeah that was my first thought as well. It's unfortunate that
--force-with-lease is not as well known though (it wasn't there first,
so many people picked it up and "it's good enough" to not pickup other
--force-with-foo options).

Maybe we should add the option receive.denyNonFastForwards =
onlyWithLease instead?

Thanks,
Stefan

>
> If you use `--force-with-lease`, you simply cannot force-forget anything on the remote side that you cannot undo (because you have everything locally you need to undo it).
>
> Ciao,
> Johannes
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: On undoing a forced push

[Duy Nguyen]

On Tue, Jun 9, 2015 at 11:29 PM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> Sorry to chime in so late in the discussion, but I think that the `--force-with-lease` option is what you are looking for. It allows you to force-push *but only* if the forced push would overwrite the ref we expect, i.e. (simplified, but you get the idea) `git push --force-with-lease <remote> <ref>` will *only* succeed if the remote's <ref> agrees with the local `refs/remotes/<remote>/<ref>`.
>
> If you use `--force-with-lease`, you simply cannot force-forget anything on the remote side that you cannot undo (because you have everything locally you need to undo it).

Yeah I recall Junio did something about pushes.. I was about to
suggest that we promote force-with-lease to default --force and
current --force becomes --force --force. But there's this from commit
2233ad4 (Merge branch 'jc/push-cas' - 2013-09-09) that makes me
hesitate

    The logic to choose the default implemented here is fragile
    (e.g. "git fetch" after seeing a failure will update the
    remote-tracking branch and will make the next "push" pass,
    defeating the safety pretty easily).  It is suitable only for the
    simplest workflows, and it may hurt users more than it helps them.

Either way I still want to provide an escape hatch for --force as it's
good to reduce the number of unrecoverable operations down.
-- 
Duy

Re: On undoing a forced push

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 884 bytes --]

On Tue, Jun 09, 2015 at 07:12:21PM +0700, Duy Nguyen wrote:
> diff --git a/transport.c b/transport.c
> index f080e93..6bd6a64 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -657,16 +657,17 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
>  			"[new branch]"),
>  			ref, ref->peer_ref, NULL, porcelain);
>  	else {
> -		char quickref[84];
> +		char quickref[104];

You've increased this by 20, but you're adding 40 characters to the
strcpy.  Are you sure that's enough?

Also, you might consider writing this in terms of GIT_SHA1_HEXSZ, as it
will be more obvious that this depends on that value.  If you don't now,
I will later.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 835 bytes --]

Re: On undoing a forced push

[Duy Nguyen]

On Tue, Jun 9, 2015 at 10:00 PM, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> On Tue, Jun 09, 2015 at 07:12:21PM +0700, Duy Nguyen wrote:
>> diff --git a/transport.c b/transport.c
>> index f080e93..6bd6a64 100644
>> --- a/transport.c
>> +++ b/transport.c
>> @@ -657,16 +657,17 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
>>                       "[new branch]"),
>>                       ref, ref->peer_ref, NULL, porcelain);
>>       else {
>> -             char quickref[84];
>> +             char quickref[104];
>
> You've increased this by 20, but you're adding 40 characters to the
> strcpy.  Are you sure that's enough?
>
> Also, you might consider writing this in terms of GIT_SHA1_HEXSZ, as it
> will be more obvious that this depends on that value.  If you don't now,
> I will later.

It's a demonstration patch and I didn't pay much attention. I think
converting this quickref to strbuf may be better though, when you
convert this file to object_id.
-- 
Duy

Re: On undoing a forced push

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 973 bytes --]

On Wed, Jun 10, 2015 at 09:43:34AM +0700, Duy Nguyen wrote:
> On Tue, Jun 9, 2015 at 10:00 PM, brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > You've increased this by 20, but you're adding 40 characters to the
> > strcpy.  Are you sure that's enough?
> >
> > Also, you might consider writing this in terms of GIT_SHA1_HEXSZ, as it
> > will be more obvious that this depends on that value.  If you don't now,
> > I will later.
> 
> It's a demonstration patch and I didn't pay much attention. I think
> converting this quickref to strbuf may be better though, when you
> convert this file to object_id.

Yeah, I didn't realize until after the fact that it was only supposed to
be a demo.  I agree that strbuf might be a better idea.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 835 bytes --]