{"thread":{"id":"42952","subject":"[RFC] git-format-patch: default to --from to avoid spoofed mails?","startedAt":"2016-07-28T21:12:02Z","lastAt":"2016-08-01T19:01:07Z","messageCount":19,"participants":["Josh Triplett","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"292453","messageId":"20160728211149.GA371@x","threadId":"42952","inReplyTo":null,"subject":"[RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2016-07-28T21:11:51Z","receivedAt":"2016-07-28T21:12:02Z","isPatch":false,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"When git-format-patch formats a patch authored by someone other than\nyourself, it defaults to filling in the \"From:\" field of the email from\nthe commit author. If you explicitly pass the --from option,\ngit-format-patch will instead use your own committer identity as the\n\"From:\", and then put a \"From:\" line at the top of the body if the\ncommit author differs. (git-am know to use that as the commit author\nwhen applying.)\n\nWhile git-send-email knows how to change the patch mails to use your own\naddress as \"From:\" and add a \"From:\" line to the body for the author,\nany other tool used to send emails doesn't do that. I've seen more than\na few mails sent to various mailing lists and patch review tools with a\nspoofed \"From:\" field pointing to the commit author, typically without\nthe knowledge of the author, which can lead to interesting surprises.\n\nI'd like to propose changing the default behavior of git-format-patch to\n--from (and adding a --from-author option to override, and perhaps a\nconfig setting). This will not change the output *except* when\nformatting patches authored by someone else. git-am and git-send-email\nboth handle the --from format without any issues.\n\nBefore I write such a patch: does anyone see a problem with such a\nchange?\n\n- Josh Triplett\n"},{"id":"292456","messageId":"xmqq8twlqwan.fsf@gitster.mtv.corp.google.com","threadId":"42952","inReplyTo":"20160728211149.GA371@x","subject":"Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-28T21:37:04Z","receivedAt":"2016-07-28T21:37:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh Triplett writes:\n\n> I'd like to propose changing the default behavior of git-format-patch to\n> --from (and adding a --from-author option to override, and perhaps a\n> config setting). This will not change the output *except* when\n> formatting patches authored by someone else. git-am and git-send-email\n> both handle the --from format without any issues.\n\nI see this in \"format-patch --help\":\n\n Note that this option is only useful if you are actually\n sending the emails and want to identify yourself as the\n sender, but retain the original author (and git am will\n correctly pick up the in-body header). Note also that\n git send-email already handles this transformation for\n you, and this option should not be used if you are\n feeding the result to git send-email.\n\nThe first one says \"only useful\", but it seems what it really means\nis \"it becomes no-op if you are sending your own patch anyway\". So\nthat one does not worry me. What is most worrysome is the latter\nhalf of the last sentence. Is it really \"should not be\", or is it\nmerely \"use of this option is just a waste of time, as you would get\nexactly the same result anyway\"? If it is the latter, that is fine.\n\nOne thing I absolutely do not want to see is people to start\nrepeating their own ident on in-body \"From: \" header when they send\ntheir own patch. That would waste everybody's time pointing out\n\"You do not have to do that, it merely adds noise\". As long as you\ncan guarantee that your change won't increase the rate of that, I am\nfine with the proposal.\n\nThanks.\n\n"},{"id":"292457","messageId":"20160728215603.GA22865@sigill.intra.peff.net","threadId":"42952","inReplyTo":"xmqq8twlqwan.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-28T21:56:03Z","receivedAt":"2016-07-28T21:56:12Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 28, 2016 at 02:37:04PM -0700, Junio C Hamano wrote:\n\n> Josh Triplett writes:\n> \n> > I'd like to propose changing the default behavior of git-format-patch to\n> > --from (and adding a --from-author option to override, and perhaps a\n> > config setting). This will not change the output *except* when\n> > formatting patches authored by someone else. git-am and git-send-email\n> > both handle the --from format without any issues.\n> \n> I see this in \"format-patch --help\":\n> \n> Note that this option is only useful if you are actually\n> sending the emails and want to identify yourself as the\n> sender, but retain the original author (and git am will\n> correctly pick up the in-body header). Note also that\n> git send-email already handles this transformation for\n> you, and this option should not be used if you are\n> feeding the result to git send-email.\n> \n> The first one says \"only useful\", but it seems what it really means\n> is \"it becomes no-op if you are sending your own patch anyway\". So\n> that one does not worry me. What is most worrysome is the latter\n> half of the last sentence. Is it really \"should not be\", or is it\n> merely \"use of this option is just a waste of time, as you would get\n> exactly the same result anyway\"? If it is the latter, that is fine.\n\nIt does what you want, and omits the in-body header when it would be\nredundant.\n\nI think the original reason I did not make \"--from\" the default is that\nI was worried about breaking consumers which do not know how to handle\nin-body headers. \"git am\" knows how to handle them, but if you have a\none-off script that parses only the mail headers, it will start\nclaiming you as the author of every patch.\n\nE.g., if you do:\n\n git format-patch -o output ...\n grep -hm1 ^From: output/*\n\nright now that gets you a list of patch authors. With \"--from\", it would\nreturn your name N times.\n\nThat's obviously a toy, but I wonder if people have scripts which behave\nsimilarly.\n\nAnother way to think about it is that \"--from\" is a no-brainer when you\nreally are going to email the patches (and that's why it is has always\nbeen the default behavior in git-send-email). But if you _aren't_ going\nto mail the patches, retaining the original headers is more convenient.\nIt's not clear to me how many non-mail users of format-patch there are\n(certainly rebase is one of them, but because it uses \"am\" on the\nreceiving side, I think everything should Just Work).\n\n-Peff\n"},{"id":"292460","messageId":"xmqq4m79qujr.fsf@gitster.mtv.corp.google.com","threadId":"42952","inReplyTo":"20160728215603.GA22865@sigill.intra.peff.net","subject":"Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-28T22:14:48Z","receivedAt":"2016-07-28T22:14:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> ... What is most worrysome is the latter\n>> half of the last sentence. Is it really \"should not be\", or is it\n>> merely \"use of this option is just a waste of time, as you would get\n>> exactly the same result anyway\"? If it is the latter, that is fine.\n>\n> It does what you want, and omits the in-body header when it would be\n> redundant.\n\nOK, then I would no longer be worried about that one.\n\n> I think the original reason I did not make \"--from\" the default is that\n> I was worried about breaking consumers which do not know how to handle\n> in-body headers.\n\nThat's a fair concern.\n\nSo going back to Josh's original problem description:\n\n While git-send-email knows how to change the patch mails to use your own\n address as \"From:\" and add a \"From:\" line to the body for the author,\n any other tool used to send emails doesn't do that.\n\nI wonder how these \"any other tool\" (that reads the format-patch\noutput, i.e. mbox file with one mail per file each, and sends each\nas a piece of e-mail, without paying attention who you, the tool's\nuser, are and blindly send them with the original \"From:\" and other\nheaders intact in the header part of the message) are used in the\nwild to send patch submissions. /usr/bin/mail or /usr/bin/Mail\nwould not be among them, as I suspect they would place everything in\nthe body part, and the would do so without stripping the \"From \"\nline that exists before each e-mail message.\n\n\n"},{"id":"292466","messageId":"20160728235316.GB3676@x","threadId":"42952","inReplyTo":"xmqq4m79qujr.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2016-07-28T23:53:16Z","receivedAt":"2016-07-28T23:53:30Z","isPatch":false,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"On Thu, Jul 28, 2016 at 03:14:48PM -0700, Junio C Hamano wrote:\n> Jeff King writes:\n> > I think the original reason I did not make \"--from\" the default is that\n> > I was worried about breaking consumers which do not know how to handle\n> > in-body headers.\n> \n> That's a fair concern.\n> \n> So going back to Josh's original problem description:\n> \n> While git-send-email knows how to change the patch mails to use your own\n> address as \"From:\" and add a \"From:\" line to the body for the author,\n> any other tool used to send emails doesn't do that.\n> \n> I wonder how these \"any other tool\" (that reads the format-patch\n> output, i.e. mbox file with one mail per file each, and sends each\n> as a piece of e-mail, without paying attention who you, the tool's\n> user, are and blindly send them with the original \"From:\" and other\n> headers intact in the header part of the message) are used in the\n> wild to send patch submissions. /usr/bin/mail or /usr/bin/Mail\n> would not be among them, as I suspect they would place everything in\n> the body part, and the would do so without stripping the \"From \"\n> line that exists before each e-mail message.\n\nmutt -H would be one example; I regularly use that to send mails.\n(It'll override \"From:\" if it doesn't know the address in it, which\nloses the author information entirely; it'll work fine with the --from\nformat.) git-imap-send would be another example; its behavior would vary\nby mail client. Both of those should always work fine with a mail\nproduced via --from; they'll just ignore the in-body \"From:\" and send\nthe mail. They'd tend to do the wrong thing with a mail produced\nwithout using --from though.\n\nI don't know what people who end up sending From-spoofed mails to LKML\nare using, but I've seen such mails regularly. I also get occasional\nblowback from someone who sent such mails including patches I authored\nwith my address spoofed as \"From:\". And I've also seen someone flamed\nfor sending patches to a mailing list for review with spoofed \"From:\"\naddresses.\n\nI can think of aesthetic reasons to want the non-\"--from\" format (for\ninstance, sticking patch files into a non-git-based tool like quilt or a\ndistribution packaging system, and not wanting your own email address\nincluded), but I can't think of any tool that would produce incorrect\nresults if handed the --from format. That seems like an argument for\nswitching the default, and adding a --from-author option or similar to\nget the current output.\n"},{"id":"292467","messageId":"20160729000439.GC3676@x","threadId":"42952","inReplyTo":"20160728215603.GA22865@sigill.intra.peff.net","subject":"Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2016-07-29T00:04:39Z","receivedAt":"2016-07-29T00:04:50Z","isPatch":false,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"On Thu, Jul 28, 2016 at 05:56:03PM -0400, Jeff King wrote:\n> Another way to think about it is that \"--from\" is a no-brainer when you\n> really are going to email the patches (and that's why it is has always\n> been the default behavior in git-send-email). But if you _aren't_ going\n> to mail the patches, retaining the original headers is more convenient.\n> It's not clear to me how many non-mail users of format-patch there are\n> (certainly rebase is one of them, but because it uses \"am\" on the\n> receiving side, I think everything should Just Work).\n\nI've seen various people use git format-patch to produce a patch file to\nemail around, or a patch file to commit to a patches directory in a\ndistribution package. I don't think any such tools would work\nincorrectly with the --from output, though for purely aesthetic reasons,\nI can imagine not wanting to include your own email address when not\nsending the patch by email.\n"},{"id":"292468","messageId":"20160729000507.GA3149@x","threadId":"42952","inReplyTo":"xmqq8twlqwan.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2016-07-29T00:05:07Z","receivedAt":"2016-07-29T00:05:20Z","isPatch":false,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"On Thu, Jul 28, 2016 at 02:37:04PM -0700, Junio C Hamano wrote:\n> Josh Triplett writes:\n> \n> > I'd like to propose changing the default behavior of git-format-patch to\n> > --from (and adding a --from-author option to override, and perhaps a\n> > config setting). This will not change the output *except* when\n> > formatting patches authored by someone else. git-am and git-send-email\n> > both handle the --from format without any issues.\n> \n> I see this in \"format-patch --help\":\n> \n> Note that this option is only useful if you are actually\n> sending the emails and want to identify yourself as the\n> sender, but retain the original author (and git am will\n> correctly pick up the in-body header). Note also that\n> git send-email already handles this transformation for\n> you, and this option should not be used if you are\n> feeding the result to git send-email.\n> \n> The first one says \"only useful\", but it seems what it really means\n> is \"it becomes no-op if you are sending your own patch anyway\". So\n> that one does not worry me. What is most worrysome is the latter\n> half of the last sentence. Is it really \"should not be\", or is it\n> merely \"use of this option is just a waste of time, as you would get\n> exactly the same result anyway\"? If it is the latter, that is fine.\n\nAs far as I can tell, it's the latter. git send-email can do this same\ntransformation, but handles mails that already have the transformation\ndone to them without any issue.\n\n> One thing I absolutely do not want to see is people to start\n> repeating their own ident on in-body \"From: \" header when they send\n> their own patch. That would waste everybody's time pointing out\n> \"You do not have to do that, it merely adds noise\". As long as you\n> can guarantee that your change won't increase the rate of that, I am\n> fine with the proposal.\n\ngit format-patch with --from *only* adds an in-body \"From:\" if the\ncommit author differs from the local committer identity. So, as far as\nI can tell, the only scenario that would produce additional in-body \"From:\"\nheaders here would be if someone had failed to configure their git\nidentity, and manually set the author for their own commits. (In which\ncase, they'd also have a broken \"From:\" in any cover letter they\ngenerated.)\n\nSo, it seems exceedingly unlikely to me that this would result in\nunnecessary in-body \"From:\" headers.\n\n- Josh Triplett\n"},{"id":"292469","messageId":"20160729001618.GA9646@sigill.intra.peff.net","threadId":"42952","inReplyTo":"xmqq4m79qujr.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-29T00:16:19Z","receivedAt":"2016-07-29T00:16:27Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 28, 2016 at 03:14:48PM -0700, Junio C Hamano wrote:\n\n> > I think the original reason I did not make \"--from\" the default is that\n> > I was worried about breaking consumers which do not know how to handle\n> > in-body headers.\n> \n> That's a fair concern.\n> \n> So going back to Josh's original problem description:\n> \n> While git-send-email knows how to change the patch mails to use your own\n> address as \"From:\" and add a \"From:\" line to the body for the author,\n> any other tool used to send emails doesn't do that.\n> \n> I wonder how these \"any other tool\" (that reads the format-patch\n> output, i.e. mbox file with one mail per file each, and sends each\n> as a piece of e-mail, without paying attention who you, the tool's\n> user, are and blindly send them with the original \"From:\" and other\n> headers intact in the header part of the message) are used in the\n> wild to send patch submissions. /usr/bin/mail or /usr/bin/Mail\n> would not be among them, as I suspect they would place everything in\n> the body part, and the would do so without stripping the \"From \"\n> line that exists before each e-mail message.\n\nI cannot speak for everybody, of course, but the reason I implemented\n\"--from\" is because my workflow is basically:\n\n git format-patch --from --stdout @{u}..HEAD >mbox\n mutt -f mbox\n\nand then I use mutt's \"resend\" command to send each one. Mutt uses the\n\"From\" header written by format-patch as the default (and so I would\nhave to manually move the headers around if not for \"--from\").\n\nThe commands above are wrapped in a script, so I have no problem\nremembering to type \"--from\", but I can see how it would be irritating\nfor general use. I would go so far as to say that any time the patches\nare going to be mailed, that \"--from\" is the right thing to do (because\notherwise you are relying on your MUA to avoid impersonating the\noriginal author).\n\nThe question in my mind is whether people actually use format-patch for\nthings besides emailing, and if the final destination is something other\nthan \"git am\". It is a handy format because it is the least-lossy way\nto move commits around external to git itself. That's why \"rebase\" used\nit originally. If the final destination is \"am\" (as it is for rebase),\nthen in-body headers are OK, because we know it understands those. If\nnot, then it's a regression.\n\nI think on the whole that defaulting to \"--from\" would help more people\nthan hurt them, but if we do believe there are scripts that would be\nregressed, it probably needs a deprecation period.\n\n-Peff\n"},{"id":"292470","messageId":"20160729001756.GB9646@sigill.intra.peff.net","threadId":"42952","inReplyTo":"20160728235316.GB3676@x","subject":"Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-29T00:17:57Z","receivedAt":"2016-07-29T00:18:05Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 28, 2016 at 04:53:16PM -0700, Josh Triplett wrote:\n\n> I can think of aesthetic reasons to want the non-\"--from\" format (for\n> instance, sticking patch files into a non-git-based tool like quilt or a\n> distribution packaging system, and not wanting your own email address\n> included), but I can't think of any tool that would produce incorrect\n> results if handed the --from format. That seems like an argument for\n> switching the default, and adding a --from-author option or similar to\n> get the current output.\n\nI covered most of my view elsewhere in the thread, but I want to be\nclear that I also do not know of any widespread tool that would have a\nproblem. I am mostly worried about breaking people's private scripts.\n\n-Peff\n"},{"id":"292484","messageId":"20160729020801.GA14892@x","threadId":"42952","inReplyTo":"20160729001618.GA9646@sigill.intra.peff.net","subject":"Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2016-07-29T02:08:02Z","receivedAt":"2016-07-29T02:08:15Z","isPatch":false,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"On Thu, Jul 28, 2016 at 08:16:19PM -0400, Jeff King wrote:\n> The question in my mind is whether people actually use format-patch for\n> things besides emailing, and if the final destination is something other\n> than \"git am\". It is a handy format because it is the least-lossy way\n> to move commits around external to git itself. That's why \"rebase\" used\n> it originally. If the final destination is \"am\" (as it is for rebase),\n> then in-body headers are OK, because we know it understands those. If\n> not, then it's a regression.\n> \n> I think on the whole that defaulting to \"--from\" would help more people\n> than hurt them, but if we do believe there are scripts that would be\n> regressed, it probably needs a deprecation period.\n\nI don't think it's likely that there are scripts that would be regressed\n(and I think it's likely that there are scripts that would be\nprogressed), but I'd also have no objection to a deprecation period.\n\nI just confirmed that with the default changed, --no-from works to\nreturn to the current behavior, so we don't need a new option. And\n--no-from has worked for a long time, so scripts won't need to care if\nthey're working with an old version of git.\n\nI can provide a patch implementing a new config option to set the\nformat-patch --from default (\"false\" for --no-from, \"true\" for --from,\nor a string value for --from=value).\n\nDo you think this needs the kind of very noisy deprecation period that\npush.default had, where anyone without the git-config option set gets a\nwarning to stderr? Or do you think it would suffice to provide a\nwarning in the release notes for a while and then change the default?\n"},{"id":"292532","messageId":"xmqqfuqspemy.fsf@gitster.mtv.corp.google.com","threadId":"42952","inReplyTo":"20160729000507.GA3149@x","subject":"Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-29T16:56:05Z","receivedAt":"2016-07-29T16:56:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh Triplett writes:\n\n> So, it seems exceedingly unlikely to me that this would result in\n> unnecessary in-body \"From:\" headers.\n\nYup, Peff elsewhere on the thread gave the same conclusion, and\nafter reading the codepaths involved again, I agree.\n"},{"id":"292571","messageId":"20160729225800.GA23268@sigill.intra.peff.net","threadId":"42952","inReplyTo":"20160729020801.GA14892@x","subject":"Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-29T22:58:00Z","receivedAt":"2016-07-29T22:58:10Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 28, 2016 at 07:08:02PM -0700, Josh Triplett wrote:\n\n> > I think on the whole that defaulting to \"--from\" would help more people\n> > than hurt them, but if we do believe there are scripts that would be\n> > regressed, it probably needs a deprecation period.\n> \n> I don't think it's likely that there are scripts that would be regressed\n> (and I think it's likely that there are scripts that would be\n> progressed), but I'd also have no objection to a deprecation period.\n\nI'm on the fence, so I'd leave the final decision on whether to deal\nwith deprecation to you (who will write the patch) and Junio (as the\nmaintainer).\n\n> I just confirmed that with the default changed, --no-from works to\n> return to the current behavior, so we don't need a new option. And\n> --no-from has worked for a long time, so scripts won't need to care if\n> they're working with an old version of git.\n> \n> I can provide a patch implementing a new config option to set the\n> format-patch --from default (\"false\" for --no-from, \"true\" for --from,\n> or a string value for --from=value).\n\nI'd be surprised if the config option is actually useful to anybody in\npractice (the distinction is not \"this my preference\" as much as \"in\nwhat context am I calling the program?\"). It is a convenient way to do\ndeprecations (introduce an option, then flip the default, leaving the\noption as an escape hatch for anybody who has been broken), though.\n\n> Do you think this needs the kind of very noisy deprecation period that\n> push.default had, where anyone without the git-config option set gets a\n> warning to stderr? Or do you think it would suffice to provide a\n> warning in the release notes for a while and then change the default?\n\nIMHO the noisy deprecations haven't really been all that beneficial.\nEven after the length push.default one, I think we still had people who\nhad skipped enough versions to miss it, and quite a few people became\nconfused or annoyed by the spew of text.\n\nOTOH, I'm not sure most people read the release notes carefully, either.\nIt would be nice if we could make the change and count on people to\nnotice it in 'next' or even 'master' before the release, but empirically\nthat does not happen.\n\nSo I dunno. If we consider the risk minor, perhaps a mention in the\nrelease notes for version X, and then the change in X+1 would be fine.\nThat gives some opportunity for release-note readers to pipe up. It's\nnot foolproof, but it would give us more confidence.\n\n-Peff\n"},{"id":"292588","messageId":"20160730045055.e3t6tpgi6zch4epo@x","threadId":"42952","inReplyTo":"20160729225800.GA23268@sigill.intra.peff.net","subject":"Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2016-07-30T04:50:55Z","receivedAt":"2016-07-30T04:51:22Z","isPatch":false,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"On Fri, Jul 29, 2016 at 06:58:00PM -0400, Jeff King wrote:\n> On Thu, Jul 28, 2016 at 07:08:02PM -0700, Josh Triplett wrote:\n> \n> > > I think on the whole that defaulting to \"--from\" would help more people\n> > > than hurt them, but if we do believe there are scripts that would be\n> > > regressed, it probably needs a deprecation period.\n> > \n> > I don't think it's likely that there are scripts that would be regressed\n> > (and I think it's likely that there are scripts that would be\n> > progressed), but I'd also have no objection to a deprecation period.\n> \n> I'm on the fence, so I'd leave the final decision on whether to deal\n> with deprecation to you (who will write the patch) and Junio (as the\n> maintainer).\n\nOK, see the plan proposed below then.\n\n> > I just confirmed that with the default changed, --no-from works to\n> > return to the current behavior, so we don't need a new option. And\n> > --no-from has worked for a long time, so scripts won't need to care if\n> > they're working with an old version of git.\n> > \n> > I can provide a patch implementing a new config option to set the\n> > format-patch --from default (\"false\" for --no-from, \"true\" for --from,\n> > or a string value for --from=value).\n> \n> I'd be surprised if the config option is actually useful to anybody in\n> practice (the distinction is not \"this my preference\" as much as \"in\n> what context am I calling the program?\"). It is a convenient way to do\n> deprecations (introduce an option, then flip the default, leaving the\n> option as an escape hatch for anybody who has been broken), though.\n\nIt also allows people to change their local default in advance of git\nchanging the default.\n\n> > Do you think this needs the kind of very noisy deprecation period that\n> > push.default had, where anyone without the git-config option set gets a\n> > warning to stderr? Or do you think it would suffice to provide a\n> > warning in the release notes for a while and then change the default?\n> \n> IMHO the noisy deprecations haven't really been all that beneficial.\n> Even after the length push.default one, I think we still had people who\n> had skipped enough versions to miss it, and quite a few people became\n> confused or annoyed by the spew of text.\n> \n> OTOH, I'm not sure most people read the release notes carefully, either.\n> It would be nice if we could make the change and count on people to\n> notice it in 'next' or even 'master' before the release, but empirically\n> that does not happen.\n> \n> So I dunno. If we consider the risk minor, perhaps a mention in the\n> release notes for version X, and then the change in X+1 would be fine.\n> That gives some opportunity for release-note readers to pipe up. It's\n> not foolproof, but it would give us more confidence.\n\nI would propose the following then:\n\n- I'll write a patch adding a config option format.from, along with\n documentation, without changing the default.\n- The release notes for the version of git introducing that config\n option should mention, prominently, the plan to change the default in\n a future version of git.\n- A subsequent release can change the default. No major rush to do\n this.\n\nDoes that sound reasonable?\n\n- Josh Triplett\n"},{"id":"292589","messageId":"20160730054742.iwmr4pavdy5z73rw@sigill.intra.peff.net","threadId":"42952","inReplyTo":"20160730045055.e3t6tpgi6zch4epo@x","subject":"Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-30T05:47:42Z","receivedAt":"2016-07-30T05:47:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 29, 2016 at 09:50:55PM -0700, Josh Triplett wrote:\n\n> I would propose the following then:\n> \n> - I'll write a patch adding a config option format.from, along with\n> documentation, without changing the default.\n> - The release notes for the version of git introducing that config\n> option should mention, prominently, the plan to change the default in\n> a future version of git.\n> - A subsequent release can change the default. No major rush to do\n> this.\n> \n> Does that sound reasonable?\n\nThat sounds fine to me.\n\nI do have to admit that after reading through the \"format.*\" section of\ngit-config(1), there is quite a bit that is configurable in it. So\nperhaps we do not need to be as careful about behavior changes as I\nthought.\n\nSo if you wanted to squish steps 2 and 3 together, that would also be OK\nby me.\n\n-Peff\n"},{"id":"292590","messageId":"20160730055717.i7rtgswbi6q2cdsg@x","threadId":"42952","inReplyTo":"20160730054742.iwmr4pavdy5z73rw@sigill.intra.peff.net","subject":"Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2016-07-30T05:57:17Z","receivedAt":"2016-07-30T05:57:37Z","isPatch":false,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"On Sat, Jul 30, 2016 at 01:47:42AM -0400, Jeff King wrote:\n> On Fri, Jul 29, 2016 at 09:50:55PM -0700, Josh Triplett wrote:\n> \n> > I would propose the following then:\n> > \n> > - I'll write a patch adding a config option format.from, along with\n> > documentation, without changing the default.\n> > - The release notes for the version of git introducing that config\n> > option should mention, prominently, the plan to change the default in\n> > a future version of git.\n> > - A subsequent release can change the default. No major rush to do\n> > this.\n> > \n> > Does that sound reasonable?\n> \n> That sounds fine to me.\n> \n> I do have to admit that after reading through the \"format.*\" section of\n> git-config(1), there is quite a bit that is configurable in it. So\n> perhaps we do not need to be as careful about behavior changes as I\n> thought.\n> \n> So if you wanted to squish steps 2 and 3 together, that would also be OK\n> by me.\n\nI'll send two separate patches, and I'll leave it up to Junio whether to\napply the second one right away or wait a release.\n"},{"id":"292593","messageId":"20160730094144.ajtkdveuhpf5hmvg@x","threadId":"42952","inReplyTo":"20160730055717.i7rtgswbi6q2cdsg@x","subject":"[PATCH 0/2] format-patch: Transition the default to --from to avoid spoofed mails","fromName":"Josh Triplett","fromEmail":"josh@joshtriplett.org","sentAt":"2016-07-30T09:41:44Z","receivedAt":"2016-07-30T09:42:02Z","isPatch":true,"sender":{"key":"josh@joshtriplett.org","avatar":"https://avatars.githubusercontent.com/u/162737?v=4"},"body":"As discussed, this patch series allows transitioning the default\nbehavior of format-patch to --from, to avoid spoofing mails when\nformatting commits not written by the user.\n\nThe first patch introduces the format.from option to set the default\nvalue of format-patch --from. This patch doesn't change the default\nbehavior of format-patch, so it can go in without any transition.\n\nThe second patch changes the default to --from. If you'd like to delay\nthis patch for a release and mention the planned change in the release\nnotes, let me know and I'll provide text for the release notes; if you\ndon't think this needs a transition period, you can go ahead and apply\nthe second patch.\n\nJosh Triplett (2):\n format-patch: Add a config option format.from to set the default for --from\n format-patch: Default to --from\n\n Documentation/config.txt | 10 ++++-\n builtin/log.c | 14 +++++-\n contrib/completion/git-completion.bash | 1 +-\n t/t4014-format-patch.sh | 68 +++++++++++++++++++++++++--\n 4 files changed, 89 insertions(+), 4 deletions(-)\n\n-- \ngit-series 0.8.7\n"},{"id":"292758","messageId":"xmqqfuqol7dx.fsf@gitster.mtv.corp.google.com","threadId":"42952","inReplyTo":"20160730054742.iwmr4pavdy5z73rw@sigill.intra.peff.net","subject":"Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-01T17:35:22Z","receivedAt":"2016-08-01T17:40:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Jul 29, 2016 at 09:50:55PM -0700, Josh Triplett wrote:\n>\n>> I would propose the following then:\n>> \n>> - I'll write a patch adding a config option format.from, along with\n>> documentation, without changing the default.\n>> - The release notes for the version of git introducing that config\n>> option should mention, prominently, the plan to change the default in\n>> a future version of git.\n>> - A subsequent release can change the default. No major rush to do\n>> this.\n>> \n>> Does that sound reasonable?\n>\n> That sounds fine to me.\n\nTo me, too.\n\n> I do have to admit that after reading through the \"format.*\" section of\n> git-config(1), there is quite a bit that is configurable in it. So\n> perhaps we do not need to be as careful about behavior changes as I\n> thought.\n\nI am not sure how the first sentence (which I agree with; a random\nuser can have quite a different behaviour configured when the\ncommand is run without any option) leads to the conclusion in the\nsecond sentence. The user can break assumptions made by a tool that\nreads format-patch output by tweaking his config but at least he\nknows that he changed the configuration, i.e. the breakage can be\nexplained and attributed to his own action. The change in the\ndefault is somewhat different.\n\nWhen we _know_ we are going to be changing the default, we should\nforewarn in previous releases (in release notes, and perhaps we\nwould want to have a runtime warning when the user formats others'\nchanges without having format.from explicitly set to either true or\nfalse).\n\nSo the second step can be delayed and does not have to be done for\nthe release that includes the first change. But I am not sure how\n\"there are many format.* configuration\" leads to \"we just announce\nthat we changed the default and tell peole there is a new knob to\nretain the original behaviour\".\n\n> So if you wanted to squish steps 2 and 3 together, that would also be OK\n> by me.\n"},{"id":"292759","messageId":"20160801174316.4boo4slop77mtry4@sigill.intra.peff.net","threadId":"42952","inReplyTo":"xmqqfuqol7dx.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-01T17:43:16Z","receivedAt":"2016-08-01T17:43:23Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 01, 2016 at 10:35:22AM -0700, Junio C Hamano wrote:\n\n> > I do have to admit that after reading through the \"format.*\" section of\n> > git-config(1), there is quite a bit that is configurable in it. So\n> > perhaps we do not need to be as careful about behavior changes as I\n> > thought.\n> \n> I am not sure how the first sentence (which I agree with; a random\n> user can have quite a different behaviour configured when the\n> command is run without any option) leads to the conclusion in the\n> second sentence. The user can break assumptions made by a tool that\n> reads format-patch output by tweaking his config but at least he\n> knows that he changed the configuration, i.e. the breakage can be\n> explained and attributed to his own action. The change in the\n> default is somewhat different.\n\nI half-agree. Config that causes unpredictable behavior can break\nsomebody else's script that you are running. If you say \"oh, I guess I\nshouldn't set that config\" and move on with your life, then the config\nhasn't really hurt anybody. If you complain to the script author that\ntheir script is broken, and insist that they pass the --no-foo option,\nthen the script writer does not care much whether it was a config option\nor a change of default.\n\n-Peff\n\nPS We also changed the default pager behavior for format-patch recently,\n though I can't actually think of any script regressions that would\n cause, since it only kicks in when output is going to the tty.\n"},{"id":"292775","messageId":"xmqqfuqojowp.fsf@gitster.mtv.corp.google.com","threadId":"42952","inReplyTo":"20160801174316.4boo4slop77mtry4@sigill.intra.peff.net","subject":"Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-01T18:59:50Z","receivedAt":"2016-08-01T19:01:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I half-agree. Config that causes unpredictable behavior can break\n> somebody else's script that you are running. If you say \"oh, I guess I\n> shouldn't set that config\" and move on with your life, then the config\n> hasn't really hurt anybody. If you complain to the script author that\n> their script is broken, and insist that they pass the --no-foo option,\n> then the script writer does not care much whether it was a config option\n> or a change of default.\n\nHmph, I never considered that POV but you are right.\n\nI am not sure if that argues that we have to be even more careful\nnot to add non-essential configuration variable, or that we can\nafford to be much less careful when changing the default, though.\nA little bit of both, I guess.\n\n"}]}