git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [RFC] git-format-patch: default to --from to avoid spoofed mails?
@ 2016-07-28 21:11 Josh Triplett
  2016-07-28 21:37 ` Junio C Hamano
  0 siblings, 1 reply; 19+ messages in thread
From: Josh Triplett @ 2016-07-28 21:11 UTC (permalink / raw)
  To: git

When git-format-patch formats a patch authored by someone other than
yourself, it defaults to filling in the "From:" field of the email from
the commit author.  If you explicitly pass the --from option,
git-format-patch will instead use your own committer identity as the
"From:", and then put a "From:" line at the top of the body if the
commit author differs.  (git-am know to use that as the commit author
when applying.)

While git-send-email knows how to change the patch mails to use your own
address as "From:" and add a "From:" line to the body for the author,
any other tool used to send emails doesn't do that.  I've seen more than
a few mails sent to various mailing lists and patch review tools with a
spoofed "From:" field pointing to the commit author, typically without
the knowledge of the author, which can lead to interesting surprises.

I'd like to propose changing the default behavior of git-format-patch to
--from (and adding a --from-author option to override, and perhaps a
config setting).  This will not change the output *except* when
formatting patches authored by someone else.  git-am and git-send-email
both handle the --from format without any issues.

Before I write such a patch: does anyone see a problem with such a
change?

- Josh Triplett

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?
  2016-07-28 21:11 [RFC] git-format-patch: default to --from to avoid spoofed mails? Josh Triplett
@ 2016-07-28 21:37 ` Junio C Hamano
  2016-07-28 21:56   ` Jeff King
  2016-07-29  0:05   ` Josh Triplett
  0 siblings, 2 replies; 19+ messages in thread
From: Junio C Hamano @ 2016-07-28 21:37 UTC (permalink / raw)
  To: Josh Triplett; +Cc: git

Josh Triplett <josh@joshtriplett.org> writes:

> I'd like to propose changing the default behavior of git-format-patch to
> --from (and adding a --from-author option to override, and perhaps a
> config setting).  This will not change the output *except* when
> formatting patches authored by someone else.  git-am and git-send-email
> both handle the --from format without any issues.

I see this in "format-patch --help":

           Note that this option is only useful if you are actually
           sending the emails and want to identify yourself as the
           sender, but retain the original author (and git am will
           correctly pick up the in-body header).  Note also that
           git send-email already handles this transformation for
           you, and this option should not be used if you are
           feeding the result to git send-email.

The first one says "only useful", but it seems what it really means
is "it becomes no-op if you are sending your own patch anyway".  So
that one does not worry me.  What is most worrysome is the latter
half of the last sentence.  Is it really "should not be", or is it
merely "use of this option is just a waste of time, as you would get
exactly the same result anyway"?  If it is the latter, that is fine.

One thing I absolutely do not want to see is people to start
repeating their own ident on in-body "From: " header when they send
their own patch.  That would waste everybody's time pointing out
"You do not have to do that, it merely adds noise".  As long as you
can guarantee that your change won't increase the rate of that, I am
fine with the proposal.

Thanks.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?
  2016-07-28 21:37 ` Junio C Hamano
@ 2016-07-28 21:56   ` Jeff King
  2016-07-28 22:14     ` Junio C Hamano
  2016-07-29  0:04     ` Josh Triplett
  2016-07-29  0:05   ` Josh Triplett
  1 sibling, 2 replies; 19+ messages in thread
From: Jeff King @ 2016-07-28 21:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Josh Triplett, git

On Thu, Jul 28, 2016 at 02:37:04PM -0700, Junio C Hamano wrote:

> Josh Triplett <josh@joshtriplett.org> writes:
> 
> > I'd like to propose changing the default behavior of git-format-patch to
> > --from (and adding a --from-author option to override, and perhaps a
> > config setting).  This will not change the output *except* when
> > formatting patches authored by someone else.  git-am and git-send-email
> > both handle the --from format without any issues.
> 
> I see this in "format-patch --help":
> 
>            Note that this option is only useful if you are actually
>            sending the emails and want to identify yourself as the
>            sender, but retain the original author (and git am will
>            correctly pick up the in-body header).  Note also that
>            git send-email already handles this transformation for
>            you, and this option should not be used if you are
>            feeding the result to git send-email.
> 
> The first one says "only useful", but it seems what it really means
> is "it becomes no-op if you are sending your own patch anyway".  So
> that one does not worry me.  What is most worrysome is the latter
> half of the last sentence.  Is it really "should not be", or is it
> merely "use of this option is just a waste of time, as you would get
> exactly the same result anyway"?  If it is the latter, that is fine.

It does what you want, and omits the in-body header when it would be
redundant.

I think the original reason I did not make "--from" the default is that
I was worried about breaking consumers which do not know how to handle
in-body headers. "git am" knows how to handle them, but if you have a
one-off script that parses only the mail headers, it will start
claiming you as the author of every patch.

E.g., if you do:

  git format-patch -o output ...
  grep -hm1 ^From: output/*

right now that gets you a list of patch authors. With "--from", it would
return your name N times.

That's obviously a toy, but I wonder if people have scripts which behave
similarly.

Another way to think about it is that "--from" is a no-brainer when you
really are going to email the patches (and that's why it is has always
been the default behavior in git-send-email). But if you _aren't_ going
to mail the patches, retaining the original headers is more convenient.
It's not clear to me how many non-mail users of format-patch there are
(certainly rebase is one of them, but because it uses "am" on the
receiving side, I think everything should Just Work).

-Peff

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?
  2016-07-28 21:56   ` Jeff King
@ 2016-07-28 22:14     ` Junio C Hamano
  2016-07-28 23:53       ` Josh Triplett
  2016-07-29  0:16       ` Jeff King
  2016-07-29  0:04     ` Josh Triplett
  1 sibling, 2 replies; 19+ messages in thread
From: Junio C Hamano @ 2016-07-28 22:14 UTC (permalink / raw)
  To: Jeff King; +Cc: Josh Triplett, git

Jeff King <peff@peff.net> writes:

>> ...  What is most worrysome is the latter
>> half of the last sentence.  Is it really "should not be", or is it
>> merely "use of this option is just a waste of time, as you would get
>> exactly the same result anyway"?  If it is the latter, that is fine.
>
> It does what you want, and omits the in-body header when it would be
> redundant.

OK, then I would no longer be worried about that one.

> I think the original reason I did not make "--from" the default is that
> I was worried about breaking consumers which do not know how to handle
> in-body headers.

That's a fair concern.

So going back to Josh's original problem description:

    While git-send-email knows how to change the patch mails to use your own
    address as "From:" and add a "From:" line to the body for the author,
    any other tool used to send emails doesn't do that.

I wonder how these "any other tool" (that reads the format-patch
output, i.e. mbox file with one mail per file each, and sends each
as a piece of e-mail, without paying attention who you, the tool's
user, are and blindly send them with the original "From:" and other
headers intact in the header part of the message) are used in the
wild to send patch submissions.  /usr/bin/mail or /usr/bin/Mail
would not be among them, as I suspect they would place everything in
the body part, and the would do so without stripping the "From "
line that exists before each e-mail message.



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?
  2016-07-28 22:14     ` Junio C Hamano
@ 2016-07-28 23:53       ` Josh Triplett
  2016-07-29  0:17         ` Jeff King
  2016-07-29  0:16       ` Jeff King
  1 sibling, 1 reply; 19+ messages in thread
From: Josh Triplett @ 2016-07-28 23:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

On Thu, Jul 28, 2016 at 03:14:48PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
> > I think the original reason I did not make "--from" the default is that
> > I was worried about breaking consumers which do not know how to handle
> > in-body headers.
> 
> That's a fair concern.
> 
> So going back to Josh's original problem description:
> 
>     While git-send-email knows how to change the patch mails to use your own
>     address as "From:" and add a "From:" line to the body for the author,
>     any other tool used to send emails doesn't do that.
> 
> I wonder how these "any other tool" (that reads the format-patch
> output, i.e. mbox file with one mail per file each, and sends each
> as a piece of e-mail, without paying attention who you, the tool's
> user, are and blindly send them with the original "From:" and other
> headers intact in the header part of the message) are used in the
> wild to send patch submissions.  /usr/bin/mail or /usr/bin/Mail
> would not be among them, as I suspect they would place everything in
> the body part, and the would do so without stripping the "From "
> line that exists before each e-mail message.

mutt -H would be one example; I regularly use that to send mails.
(It'll override "From:" if it doesn't know the address in it, which
loses the author information entirely; it'll work fine with the --from
format.) git-imap-send would be another example; its behavior would vary
by mail client.  Both of those should always work fine with a mail
produced via --from; they'll just ignore the in-body "From:" and send
the mail.  They'd tend to do the wrong thing with a mail produced
without using --from though.

I don't know what people who end up sending From-spoofed mails to LKML
are using, but I've seen such mails regularly.  I also get occasional
blowback from someone who sent such mails including patches I authored
with my address spoofed as "From:".  And I've also seen someone flamed
for sending patches to a mailing list for review with spoofed "From:"
addresses.

I can think of aesthetic reasons to want the non-"--from" format (for
instance, sticking patch files into a non-git-based tool like quilt or a
distribution packaging system, and not wanting your own email address
included), but I can't think of any tool that would produce incorrect
results if handed the --from format.  That seems like an argument for
switching the default, and adding a --from-author option or similar to
get the current output.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?
  2016-07-28 21:56   ` Jeff King
  2016-07-28 22:14     ` Junio C Hamano
@ 2016-07-29  0:04     ` Josh Triplett
  1 sibling, 0 replies; 19+ messages in thread
From: Josh Triplett @ 2016-07-29  0:04 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git

On Thu, Jul 28, 2016 at 05:56:03PM -0400, Jeff King wrote:
> Another way to think about it is that "--from" is a no-brainer when you
> really are going to email the patches (and that's why it is has always
> been the default behavior in git-send-email). But if you _aren't_ going
> to mail the patches, retaining the original headers is more convenient.
> It's not clear to me how many non-mail users of format-patch there are
> (certainly rebase is one of them, but because it uses "am" on the
> receiving side, I think everything should Just Work).

I've seen various people use git format-patch to produce a patch file to
email around, or a patch file to commit to a patches directory in a
distribution package.  I don't think any such tools would work
incorrectly with the --from output, though for purely aesthetic reasons,
I can imagine not wanting to include your own email address when not
sending the patch by email.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?
  2016-07-28 21:37 ` Junio C Hamano
  2016-07-28 21:56   ` Jeff King
@ 2016-07-29  0:05   ` Josh Triplett
  2016-07-29 16:56     ` Junio C Hamano
  1 sibling, 1 reply; 19+ messages in thread
From: Josh Triplett @ 2016-07-29  0:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Thu, Jul 28, 2016 at 02:37:04PM -0700, Junio C Hamano wrote:
> Josh Triplett <josh@joshtriplett.org> writes:
> 
> > I'd like to propose changing the default behavior of git-format-patch to
> > --from (and adding a --from-author option to override, and perhaps a
> > config setting).  This will not change the output *except* when
> > formatting patches authored by someone else.  git-am and git-send-email
> > both handle the --from format without any issues.
> 
> I see this in "format-patch --help":
> 
>            Note that this option is only useful if you are actually
>            sending the emails and want to identify yourself as the
>            sender, but retain the original author (and git am will
>            correctly pick up the in-body header).  Note also that
>            git send-email already handles this transformation for
>            you, and this option should not be used if you are
>            feeding the result to git send-email.
> 
> The first one says "only useful", but it seems what it really means
> is "it becomes no-op if you are sending your own patch anyway".  So
> that one does not worry me.  What is most worrysome is the latter
> half of the last sentence.  Is it really "should not be", or is it
> merely "use of this option is just a waste of time, as you would get
> exactly the same result anyway"?  If it is the latter, that is fine.

As far as I can tell, it's the latter.  git send-email can do this same
transformation, but handles mails that already have the transformation
done to them without any issue.

> One thing I absolutely do not want to see is people to start
> repeating their own ident on in-body "From: " header when they send
> their own patch.  That would waste everybody's time pointing out
> "You do not have to do that, it merely adds noise".  As long as you
> can guarantee that your change won't increase the rate of that, I am
> fine with the proposal.

git format-patch with --from *only* adds an in-body "From:" if the
commit author differs from the local committer identity.  So, as far as
I can tell, the only scenario that would produce additional in-body "From:"
headers here would be if someone had failed to configure their git
identity, and manually set the author for their own commits.  (In which
case, they'd also have a broken "From:" in any cover letter they
generated.)

So, it seems exceedingly unlikely to me that this would result in
unnecessary in-body "From:" headers.

- Josh Triplett

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?
  2016-07-28 22:14     ` Junio C Hamano
  2016-07-28 23:53       ` Josh Triplett
@ 2016-07-29  0:16       ` Jeff King
  2016-07-29  2:08         ` Josh Triplett
  1 sibling, 1 reply; 19+ messages in thread
From: Jeff King @ 2016-07-29  0:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Josh Triplett, git

On Thu, Jul 28, 2016 at 03:14:48PM -0700, Junio C Hamano wrote:

> > I think the original reason I did not make "--from" the default is that
> > I was worried about breaking consumers which do not know how to handle
> > in-body headers.
> 
> That's a fair concern.
> 
> So going back to Josh's original problem description:
> 
>     While git-send-email knows how to change the patch mails to use your own
>     address as "From:" and add a "From:" line to the body for the author,
>     any other tool used to send emails doesn't do that.
> 
> I wonder how these "any other tool" (that reads the format-patch
> output, i.e. mbox file with one mail per file each, and sends each
> as a piece of e-mail, without paying attention who you, the tool's
> user, are and blindly send them with the original "From:" and other
> headers intact in the header part of the message) are used in the
> wild to send patch submissions.  /usr/bin/mail or /usr/bin/Mail
> would not be among them, as I suspect they would place everything in
> the body part, and the would do so without stripping the "From "
> line that exists before each e-mail message.

I cannot speak for everybody, of course, but the reason I implemented
"--from" is because my workflow is basically:

  git format-patch --from --stdout @{u}..HEAD >mbox
  mutt -f mbox

and then I use mutt's "resend" command to send each one. Mutt uses the
"From" header written by format-patch as the default (and so I would
have to manually move the headers around if not for "--from").

The commands above are wrapped in a script, so I have no problem
remembering to type "--from", but I can see how it would be irritating
for general use. I would go so far as to say that any time the patches
are going to be mailed, that "--from" is the right thing to do (because
otherwise you are relying on your MUA to avoid impersonating the
original author).

The question in my mind is whether people actually use format-patch for
things besides emailing, and if the final destination is something other
than "git am".  It is a handy format because it is the least-lossy way
to move commits around external to git itself.  That's why "rebase" used
it originally. If the final destination is "am" (as it is for rebase),
then in-body headers are OK, because we know it understands those. If
not, then it's a regression.

I think on the whole that defaulting to "--from" would help more people
than hurt them, but if we do believe there are scripts that would be
regressed, it probably needs a deprecation period.

-Peff

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?
  2016-07-28 23:53       ` Josh Triplett
@ 2016-07-29  0:17         ` Jeff King
  0 siblings, 0 replies; 19+ messages in thread
From: Jeff King @ 2016-07-29  0:17 UTC (permalink / raw)
  To: Josh Triplett; +Cc: Junio C Hamano, git

On Thu, Jul 28, 2016 at 04:53:16PM -0700, Josh Triplett wrote:

> I can think of aesthetic reasons to want the non-"--from" format (for
> instance, sticking patch files into a non-git-based tool like quilt or a
> distribution packaging system, and not wanting your own email address
> included), but I can't think of any tool that would produce incorrect
> results if handed the --from format.  That seems like an argument for
> switching the default, and adding a --from-author option or similar to
> get the current output.

I covered most of my view elsewhere in the thread, but I want to be
clear that I also do not know of any widespread tool that would have a
problem. I am mostly worried about breaking people's private scripts.

-Peff

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?
  2016-07-29  0:16       ` Jeff King
@ 2016-07-29  2:08         ` Josh Triplett
  2016-07-29 22:58           ` Jeff King
  0 siblings, 1 reply; 19+ messages in thread
From: Josh Triplett @ 2016-07-29  2:08 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git

On Thu, Jul 28, 2016 at 08:16:19PM -0400, Jeff King wrote:
> The question in my mind is whether people actually use format-patch for
> things besides emailing, and if the final destination is something other
> than "git am".  It is a handy format because it is the least-lossy way
> to move commits around external to git itself.  That's why "rebase" used
> it originally. If the final destination is "am" (as it is for rebase),
> then in-body headers are OK, because we know it understands those. If
> not, then it's a regression.
> 
> I think on the whole that defaulting to "--from" would help more people
> than hurt them, but if we do believe there are scripts that would be
> regressed, it probably needs a deprecation period.

I don't think it's likely that there are scripts that would be regressed
(and I think it's likely that there are scripts that would be
progressed), but I'd also have no objection to a deprecation period.

I just confirmed that with the default changed, --no-from works to
return to the current behavior, so we don't need a new option.  And
--no-from has worked for a long time, so scripts won't need to care if
they're working with an old version of git.

I can provide a patch implementing a new config option to set the
format-patch --from default ("false" for --no-from, "true" for --from,
or a string value for --from=value).

Do you think this needs the kind of very noisy deprecation period that
push.default had, where anyone without the git-config option set gets a
warning to stderr?  Or do you think it would suffice to provide a
warning in the release notes for a while and then change the default?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?
  2016-07-29  0:05   ` Josh Triplett
@ 2016-07-29 16:56     ` Junio C Hamano
  0 siblings, 0 replies; 19+ messages in thread
From: Junio C Hamano @ 2016-07-29 16:56 UTC (permalink / raw)
  To: Josh Triplett; +Cc: git

Josh Triplett <josh@joshtriplett.org> writes:

> So, it seems exceedingly unlikely to me that this would result in
> unnecessary in-body "From:" headers.

Yup, Peff elsewhere on the thread gave the same conclusion, and
after reading the codepaths involved again, I agree.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?
  2016-07-29  2:08         ` Josh Triplett
@ 2016-07-29 22:58           ` Jeff King
  2016-07-30  4:50             ` Josh Triplett
  0 siblings, 1 reply; 19+ messages in thread
From: Jeff King @ 2016-07-29 22:58 UTC (permalink / raw)
  To: Josh Triplett; +Cc: Junio C Hamano, git

On Thu, Jul 28, 2016 at 07:08:02PM -0700, Josh Triplett wrote:

> > I think on the whole that defaulting to "--from" would help more people
> > than hurt them, but if we do believe there are scripts that would be
> > regressed, it probably needs a deprecation period.
> 
> I don't think it's likely that there are scripts that would be regressed
> (and I think it's likely that there are scripts that would be
> progressed), but I'd also have no objection to a deprecation period.

I'm on the fence, so I'd leave the final decision on whether to deal
with deprecation to you (who will write the patch) and Junio (as the
maintainer).

> I just confirmed that with the default changed, --no-from works to
> return to the current behavior, so we don't need a new option.  And
> --no-from has worked for a long time, so scripts won't need to care if
> they're working with an old version of git.
> 
> I can provide a patch implementing a new config option to set the
> format-patch --from default ("false" for --no-from, "true" for --from,
> or a string value for --from=value).

I'd be surprised if the config option is actually useful to anybody in
practice (the distinction is not "this my preference" as much as "in
what context am I calling the program?"). It is a convenient way to do
deprecations (introduce an option, then flip the default, leaving the
option as an escape hatch for anybody who has been broken), though.

> Do you think this needs the kind of very noisy deprecation period that
> push.default had, where anyone without the git-config option set gets a
> warning to stderr?  Or do you think it would suffice to provide a
> warning in the release notes for a while and then change the default?

IMHO the noisy deprecations haven't really been all that beneficial.
Even after the length push.default one, I think we still had people who
had skipped enough versions to miss it, and quite a few people became
confused or annoyed by the spew of text.

OTOH, I'm not sure most people read the release notes carefully, either.
It would be nice if we could make the change and count on people to
notice it in 'next' or even 'master' before the release, but empirically
that does not happen.

So I dunno. If we consider the risk minor, perhaps a mention in the
release notes for version X, and then the change in X+1 would be fine.
That gives some opportunity for release-note readers to pipe up. It's
not foolproof, but it would give us more confidence.

-Peff

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?
  2016-07-29 22:58           ` Jeff King
@ 2016-07-30  4:50             ` Josh Triplett
  2016-07-30  5:47               ` Jeff King
  0 siblings, 1 reply; 19+ messages in thread
From: Josh Triplett @ 2016-07-30  4:50 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git

On Fri, Jul 29, 2016 at 06:58:00PM -0400, Jeff King wrote:
> On Thu, Jul 28, 2016 at 07:08:02PM -0700, Josh Triplett wrote:
> 
> > > I think on the whole that defaulting to "--from" would help more people
> > > than hurt them, but if we do believe there are scripts that would be
> > > regressed, it probably needs a deprecation period.
> > 
> > I don't think it's likely that there are scripts that would be regressed
> > (and I think it's likely that there are scripts that would be
> > progressed), but I'd also have no objection to a deprecation period.
> 
> I'm on the fence, so I'd leave the final decision on whether to deal
> with deprecation to you (who will write the patch) and Junio (as the
> maintainer).

OK, see the plan proposed below then.

> > I just confirmed that with the default changed, --no-from works to
> > return to the current behavior, so we don't need a new option.  And
> > --no-from has worked for a long time, so scripts won't need to care if
> > they're working with an old version of git.
> > 
> > I can provide a patch implementing a new config option to set the
> > format-patch --from default ("false" for --no-from, "true" for --from,
> > or a string value for --from=value).
> 
> I'd be surprised if the config option is actually useful to anybody in
> practice (the distinction is not "this my preference" as much as "in
> what context am I calling the program?"). It is a convenient way to do
> deprecations (introduce an option, then flip the default, leaving the
> option as an escape hatch for anybody who has been broken), though.

It also allows people to change their local default in advance of git
changing the default.

> > Do you think this needs the kind of very noisy deprecation period that
> > push.default had, where anyone without the git-config option set gets a
> > warning to stderr?  Or do you think it would suffice to provide a
> > warning in the release notes for a while and then change the default?
> 
> IMHO the noisy deprecations haven't really been all that beneficial.
> Even after the length push.default one, I think we still had people who
> had skipped enough versions to miss it, and quite a few people became
> confused or annoyed by the spew of text.
> 
> OTOH, I'm not sure most people read the release notes carefully, either.
> It would be nice if we could make the change and count on people to
> notice it in 'next' or even 'master' before the release, but empirically
> that does not happen.
> 
> So I dunno. If we consider the risk minor, perhaps a mention in the
> release notes for version X, and then the change in X+1 would be fine.
> That gives some opportunity for release-note readers to pipe up. It's
> not foolproof, but it would give us more confidence.

I would propose the following then:

- I'll write a patch adding a config option format.from, along with
  documentation, without changing the default.
- The release notes for the version of git introducing that config
  option should mention, prominently, the plan to change the default in
  a future version of git.
- A subsequent release can change the default.  No major rush to do
  this.

Does that sound reasonable?

- Josh Triplett

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?
  2016-07-30  4:50             ` Josh Triplett
@ 2016-07-30  5:47               ` Jeff King
  2016-07-30  5:57                 ` Josh Triplett
  2016-08-01 17:35                 ` [RFC] git-format-patch: default to --from to avoid spoofed mails? Junio C Hamano
  0 siblings, 2 replies; 19+ messages in thread
From: Jeff King @ 2016-07-30  5:47 UTC (permalink / raw)
  To: Josh Triplett; +Cc: Junio C Hamano, git

On Fri, Jul 29, 2016 at 09:50:55PM -0700, Josh Triplett wrote:

> I would propose the following then:
> 
> - I'll write a patch adding a config option format.from, along with
>   documentation, without changing the default.
> - The release notes for the version of git introducing that config
>   option should mention, prominently, the plan to change the default in
>   a future version of git.
> - A subsequent release can change the default.  No major rush to do
>   this.
> 
> Does that sound reasonable?

That sounds fine to me.

I do have to admit that after reading through the "format.*" section of
git-config(1), there is quite a bit that is configurable in it. So
perhaps we do not need to be as careful about behavior changes as I
thought.

So if you wanted to squish steps 2 and 3 together, that would also be OK
by me.

-Peff

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?
  2016-07-30  5:47               ` Jeff King
@ 2016-07-30  5:57                 ` Josh Triplett
  2016-07-30  9:41                   ` [PATCH 0/2] format-patch: Transition the default to --from to avoid spoofed mails Josh Triplett
  2016-08-01 17:35                 ` [RFC] git-format-patch: default to --from to avoid spoofed mails? Junio C Hamano
  1 sibling, 1 reply; 19+ messages in thread
From: Josh Triplett @ 2016-07-30  5:57 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git

On Sat, Jul 30, 2016 at 01:47:42AM -0400, Jeff King wrote:
> On Fri, Jul 29, 2016 at 09:50:55PM -0700, Josh Triplett wrote:
> 
> > I would propose the following then:
> > 
> > - I'll write a patch adding a config option format.from, along with
> >   documentation, without changing the default.
> > - The release notes for the version of git introducing that config
> >   option should mention, prominently, the plan to change the default in
> >   a future version of git.
> > - A subsequent release can change the default.  No major rush to do
> >   this.
> > 
> > Does that sound reasonable?
> 
> That sounds fine to me.
> 
> I do have to admit that after reading through the "format.*" section of
> git-config(1), there is quite a bit that is configurable in it. So
> perhaps we do not need to be as careful about behavior changes as I
> thought.
> 
> So if you wanted to squish steps 2 and 3 together, that would also be OK
> by me.

I'll send two separate patches, and I'll leave it up to Junio whether to
apply the second one right away or wait a release.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 0/2] format-patch: Transition the default to --from to avoid spoofed mails
  2016-07-30  5:57                 ` Josh Triplett
@ 2016-07-30  9:41                   ` Josh Triplett
  0 siblings, 0 replies; 19+ messages in thread
From: Josh Triplett @ 2016-07-30  9:41 UTC (permalink / raw)
  To: Junio C Hamano, Jeff King, git

As discussed, this patch series allows transitioning the default
behavior of format-patch to --from, to avoid spoofing mails when
formatting commits not written by the user.

The first patch introduces the format.from option to set the default
value of format-patch --from.  This patch doesn't change the default
behavior of format-patch, so it can go in without any transition.

The second patch changes the default to --from.  If you'd like to delay
this patch for a release and mention the planned change in the release
notes, let me know and I'll provide text for the release notes; if you
don't think this needs a transition period, you can go ahead and apply
the second patch.

Josh Triplett (2):
  format-patch: Add a config option format.from to set the default for --from
  format-patch: Default to --from

 Documentation/config.txt               | 10 ++++-
 builtin/log.c                          | 14 +++++-
 contrib/completion/git-completion.bash |  1 +-
 t/t4014-format-patch.sh                | 68 +++++++++++++++++++++++++--
 4 files changed, 89 insertions(+), 4 deletions(-)

-- 
git-series 0.8.7

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?
  2016-07-30  5:47               ` Jeff King
  2016-07-30  5:57                 ` Josh Triplett
@ 2016-08-01 17:35                 ` Junio C Hamano
  2016-08-01 17:43                   ` Jeff King
  1 sibling, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2016-08-01 17:35 UTC (permalink / raw)
  To: Jeff King; +Cc: Josh Triplett, git

Jeff King <peff@peff.net> writes:

> On Fri, Jul 29, 2016 at 09:50:55PM -0700, Josh Triplett wrote:
>
>> I would propose the following then:
>> 
>> - I'll write a patch adding a config option format.from, along with
>>   documentation, without changing the default.
>> - The release notes for the version of git introducing that config
>>   option should mention, prominently, the plan to change the default in
>>   a future version of git.
>> - A subsequent release can change the default.  No major rush to do
>>   this.
>> 
>> Does that sound reasonable?
>
> That sounds fine to me.

To me, too.

> I do have to admit that after reading through the "format.*" section of
> git-config(1), there is quite a bit that is configurable in it. So
> perhaps we do not need to be as careful about behavior changes as I
> thought.

I am not sure how the first sentence (which I agree with; a random
user can have quite a different behaviour configured when the
command is run without any option) leads to the conclusion in the
second sentence.  The user can break assumptions made by a tool that
reads format-patch output by tweaking his config but at least he
knows that he changed the configuration, i.e. the breakage can be
explained and attributed to his own action.  The change in the
default is somewhat different.

When we _know_ we are going to be changing the default, we should
forewarn in previous releases (in release notes, and perhaps we
would want to have a runtime warning when the user formats others'
changes without having format.from explicitly set to either true or
false).

So the second step can be delayed and does not have to be done for
the release that includes the first change.  But I am not sure how
"there are many format.* configuration" leads to "we just announce
that we changed the default and tell peole there is a new knob to
retain the original behaviour".

> So if you wanted to squish steps 2 and 3 together, that would also be OK
> by me.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?
  2016-08-01 17:35                 ` [RFC] git-format-patch: default to --from to avoid spoofed mails? Junio C Hamano
@ 2016-08-01 17:43                   ` Jeff King
  2016-08-01 18:59                     ` Junio C Hamano
  0 siblings, 1 reply; 19+ messages in thread
From: Jeff King @ 2016-08-01 17:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Josh Triplett, git

On Mon, Aug 01, 2016 at 10:35:22AM -0700, Junio C Hamano wrote:

> > I do have to admit that after reading through the "format.*" section of
> > git-config(1), there is quite a bit that is configurable in it. So
> > perhaps we do not need to be as careful about behavior changes as I
> > thought.
> 
> I am not sure how the first sentence (which I agree with; a random
> user can have quite a different behaviour configured when the
> command is run without any option) leads to the conclusion in the
> second sentence.  The user can break assumptions made by a tool that
> reads format-patch output by tweaking his config but at least he
> knows that he changed the configuration, i.e. the breakage can be
> explained and attributed to his own action.  The change in the
> default is somewhat different.

I half-agree. Config that causes unpredictable behavior can break
somebody else's script that you are running. If you say "oh, I guess I
shouldn't set that config" and move on with your life, then the config
hasn't really hurt anybody. If you complain to the script author that
their script is broken, and insist that they pass the --no-foo option,
then the script writer does not care much whether it was a config option
or a change of default.

-Peff

PS We also changed the default pager behavior for format-patch recently,
   though I can't actually think of any script regressions that would
   cause, since it only kicks in when output is going to the tty.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] git-format-patch: default to --from to avoid spoofed mails?
  2016-08-01 17:43                   ` Jeff King
@ 2016-08-01 18:59                     ` Junio C Hamano
  0 siblings, 0 replies; 19+ messages in thread
From: Junio C Hamano @ 2016-08-01 18:59 UTC (permalink / raw)
  To: Jeff King; +Cc: Josh Triplett, git

Jeff King <peff@peff.net> writes:

> I half-agree. Config that causes unpredictable behavior can break
> somebody else's script that you are running. If you say "oh, I guess I
> shouldn't set that config" and move on with your life, then the config
> hasn't really hurt anybody. If you complain to the script author that
> their script is broken, and insist that they pass the --no-foo option,
> then the script writer does not care much whether it was a config option
> or a change of default.

Hmph, I never considered that POV but you are right.

I am not sure if that argues that we have to be even more careful
not to add non-essential configuration variable, or that we can
afford to be much less careful when changing the default, though.
A little bit of both, I guess.


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-08-01 19:01 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-28 21:11 [RFC] git-format-patch: default to --from to avoid spoofed mails? Josh Triplett
2016-07-28 21:37 ` Junio C Hamano
2016-07-28 21:56   ` Jeff King
2016-07-28 22:14     ` Junio C Hamano
2016-07-28 23:53       ` Josh Triplett
2016-07-29  0:17         ` Jeff King
2016-07-29  0:16       ` Jeff King
2016-07-29  2:08         ` Josh Triplett
2016-07-29 22:58           ` Jeff King
2016-07-30  4:50             ` Josh Triplett
2016-07-30  5:47               ` Jeff King
2016-07-30  5:57                 ` Josh Triplett
2016-07-30  9:41                   ` [PATCH 0/2] format-patch: Transition the default to --from to avoid spoofed mails Josh Triplett
2016-08-01 17:35                 ` [RFC] git-format-patch: default to --from to avoid spoofed mails? Junio C Hamano
2016-08-01 17:43                   ` Jeff King
2016-08-01 18:59                     ` Junio C Hamano
2016-07-29  0:04     ` Josh Triplett
2016-07-29  0:05   ` Josh Triplett
2016-07-29 16:56     ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).