Pull Requests via Git Push(blog.sesse.net)

16 pointsby stargrave2 lata temu8 comments

blog.sesse.net

https://blog.sesse.net/blog/tech/2024-07-15-13-04_pull_requests_via_git_push.html

3 comments

Nice! A couple of thoughts re. your "It's not perfect" which wouldn't add real extra complexity:

Using --from with git-format-patch will add the From: header to the commit message body in the standard way when emailing a patch written by someone else, which would allow you to preserve this easily without risking excitement from injection attacks into email headers.

Maybe the right behaviour for all-zero $oldsha is to quietly turn format-patch $oldsha..$newsha into format-patch --root $newsha?

Sesse__2 lata temu

By default, it seems git-send-email adds the in-body From: with the committer. (I don't know if git am picks this up, but I suppose it does.) If I add --from _after_ $oldsha..$newsha, it seems that's the git-format-patch --from, which _also_ adds something based on the headers sent by git-http-backend, so I get two From: in the body, where the first is:

From: anonymous <[email protected]:<…my IP address here…>>

I guess what I really want is Reply-To: being set based on the commit message, but I don't know if git can do that easily.

qhwudbebd2 lata temu

Ah interesting. I use git-format-patch instead of git-send-email myself, so I can re-read the resulting mbox before sending it. I always thought of git-send-email as a wrapper that just passed most options straight through to format-patch, but sounds like it does something more with --from.

I haven't read builtin/log.c (edit: and pretty.c) to check how risky the From: field actually is. It might be that it is guaranteed to be properly/safely quoted by git - but it might not be too! When I embedded a " in the author name by hand, the From: line was correctly quoted as

  From: "first\"last" <email@email>

but that's no guarantee that it's done for every malicious author/committer without inspecting the code.

[deleted]2 lata temu

codetrotter2 lata temu

    echo 'The push will now exit with an error. No commits have actually been pushed.'

Is this part so that the temporary git objects are cleaned up? Or is there some other reason that it has to exit with error?

Sesse__2 lata temu

It is so that the push doesn't actually become a push. :-) If the pre-receive hook exits with success, the push goes through and random people can write whatever they want to my repository. If it fails, the repository is left exactly as it was (including, as you point out, the quarantined temporary objects going away).

One could in theory use git namespaces to keep the new branches but not make them overwrite existing ones, but then people could store porn or whatever for free on your server and have it be served to others.

(I wrote the blog post)

codetrotter2 lata temu

Neat! Thanks for explaining :)

> keep the new branches but not make them overwrite existing ones, but then people could store porn or whatever for free on your server and have it be served to others

When your program sends the mail, do you use any external services for that (GMail, etc) or do you run a local SMTP server? How about the receiving address, is that one self-hosted or powered by a third-party?

Sesse__2 lata temu

I deliver and read mail locally. I don't think that matters a whole lot in this case, though? As long as you can send email from your server to yourself somehow, I guess you're fine.

codetrotter2 lata temu

Mainly thinking because if you use a third-party you could potentially trigger some spam metric if someone were to submit PRs full of spam, and thereby getting your domain landed on some blacklist which in turn would make more of the real mail you yourself send at risk of getting trapped in spam.

So using a local-only / fully self-hosted mail delivery solution is definitely the best way to do it IMO.