[PATCH] notification: use Sender and From header to clarify comment and pull request mails

[Mads Kiilerich]

On 06/17/2015 10:02 PM, Thomas De Schampheleire wrote:
> On Wed, Jun 17, 2015 at 12:27 AM, Mads Kiilerich <mads at kiilerich.com> wrote:
>> On 06/15/2015 04:23 PM, Thomas De Schampheleire wrote:
>>> On Mon, Jun 15, 2015 at 2:10 PM, Mads Kiilerich <mads at kiilerich.com>
>>> wrote:
>>>> On 06/14/2015 09:05 PM, Thomas De Schampheleire wrote:
>>>>> Hi Mads,
>>>>>
>>>>> On Fri, Jun 12, 2015 at 11:41 PM, Mads Kiilerich <mads at kiilerich.com>
>>>>> wrote:
>>>>>> Anyway, how about a first patch that refactors and consistently renames
>>>>>> "mail_from" to "envelope_from" (which seems to be the right technical
>>>>>> term
>>>>>> for what it is), and then another patch for setting "header_from" (or
>>>>>> reuse
>>>>>> mail_from).
>>>>> After further investigation, I don't think that 'envelope_from' is the
>>>>> right term. The envelope is something _around_ the message, the
>>>>> message being the contents _and_ the headers. These headers include
>>>>> both From: and Sender:
>>>>> There exist indeed the possibility of having an envelope sender, but
>>>>> this is not what we're dealing with here.
>>>>
>>>> Ok. Then I guess the code & patch it could use some further clarification
>>>> for dummies ;-)
>>>>
>>>> I haven't really digged into this since RFC 821 and 822 still applied,
>>>> but I
>>>> guess it is essential
>>>> * that we still have a way to configure what sender is used when
>>>> delivering
>>>> over SMTP (and thus where bounces will go)
>>> I don't understand what you're suggesting here...
>>>
>>>> * that we don't start setting sender in a way that interferes with the
>>>> normal delivery process
>>> I don't understand this either...
>>>
>>>> * that we don't make our mails more likely to be caught in spam
>>>> prevention
>>>> filters
>>>>
>>>> A first safe step might be just change the "name" part of the from (and
>>>> sender?) headers based on who triggered the mail, without changing any
>>>> actual addresses.
>>> How do you know that this is 'safe' ? Are you sure that a spam filter
>>> may not be more suspicious by seeing the same e-mail address sending
>>> mail under different names?
>>
>> It seems like we have some discrepancy in our understanding of how mail
>> works. Some brief statements, trying to outline how I see this big and
>> complex area:
>>
>> Mails are basically routed between Mail Transfer Agents routing/relaying
>> based on the envelope recipient and sender (pretty much independent of the
>> mail headers).
>>
>> Mail relays have to be careful they don't forward spam (or bounce spam to
>> forged senders). One way to do that is to verify that the mail comes from an
>> IP address that the SPF protocol designates as valid for the sender address.
>> Kallithea thus have to make sure it doesn't use for example a gmail.com as
>> envelop sender address (unless the mails from Kallithea actually are sent
>> through a google server).
> I was under the impression that this patch was not touching the
> envelope sender in any way, only adding the Sender header, but on
> closer inspection it turns out that the From: header and the envelope
> sender are treated the same way.
> Now I understand what you were trying to tell me, regarding not
> changing the envelope sender. I think we'll have to keep these
> separate: the envelope sender should be completely defined from the
> app_email_from variable and should not be changed. The From: field can
> change and become different I think.

Yes, something like that seems right. (It is easy to say "something is 
wrong", harder to say exactly _what_ is wrong, and even harder to 
suggest design or verify that something really is "right" ;-) )

If you only change the From header (and perhaps Sender - I'm not 
entirely sure what role these two should play) to reveal the name of the 
sender, it should probably just always be that way. Leaking of email 
addresses would be more controversial and should be a separate patch and 
perhaps be configurable.

>> Mail User Agents will usually not see the envelope addresses (especially
>> because they often have been rewritten) but will only show MIME headers like
>> From, To, and perhaps Sender.
>>
>> Spam filters are often a bit like IP routers/firewalls doing deep packet
>> inspection. They might look at anything using whatever heuristics works for
>> them (some also use well defined heuristics like DKIM headers). "Forging" of
>> headers is however quite common - see for example how this mailing list
>> works. Nothing can be guaranteed, but many valid uses depends on this not
>> being too strict.
>>
>> See how bitbucket sends issue comments. It uses the users full name but
>> combines it with issues-reply at bitbucket.org. That makes me quite sure the
>> same thing would work for us.
> So this means not using a separate Sender: field at all, but rather
> set only From:

Perhaps and probably. I'm not really familiar with the Sender header. I 
haven't really seen it used and don't see it solving a problem. Good 
arguments could probably convince me ;-)

>
>> Do you agree with some of the things I have said in this tread? Please try
>> to resend as a minimal change - perhaps with changes to the documentation
>> and .ini files explaining how it works with your change. Then we can
>> continue discussion based on that.
> Yes, I will:
> - split the patch: one part for the from/sender stuff, the other for
> the mail subjects
> - make sure that the envelope sender remains intact
> - use "Author <no-reply..." as From: and ditch the Sender: stuff
>
> However, it seems to me that the ini file should now have separate fields for:
> - app_email_from --> envelope sender, can be in "Name <email>" format.

??? The envelope header is what is used for actually delivering/routing 
the mail. I don't remember if it allows a name in addition to the actual 
address, but it would be pointless. So _if_ we want a way to configure 
the envelop sender, why not just make it be the actual address ... and 
name it accordingly?

> In my case, my SMTP requires this e-mail address to be a valid e-mail
> address, not some non-existing no-reply@ one.
> - noreply_email --> the no-reply e-mail address to be used, e.g.
> kallithea-noreply at company.com

But why use different addresses? It would still make a lot of sense to 
monitor such a no-reply address and catch bounces and other problems.

/Mads