Bug#809318: bts: overrides user-specified value of sendmail

[Daniel Shahaf]

Osamu Aoki wrote on Wed, Mar 09, 2016 at 23:39:14 +0900:
> control: severity -1 normal
> 
> Hi,
> 
> On Wed, Mar 09, 2016 at 02:22:04PM +0000, Daniel Shahaf wrote:
> > Osamu Aoki wrote on Tue, Mar 08, 2016 at 23:08:58 +0900:
> ...
> > There are two different issues being discussed in this thread.  One of
> > them is a wishlist item and one of them is not.
> 
> OK
>  
> > The root problem is that I ran «bts --sendmail=foo» and
> > /usr/sbin/sendmail was executed.  This can happen not only when the
> > argument contains spaces (as in my original example) but also when the
> > argument is an absolute path to an executable or a script that
> > accidentally lacks the +x permission:
> 
> If it does not have +x permission, then script is not executed as
> expected.  So what is wrong?  I do not understand.
> 

I expect the case of «bts --sendmail=value-that-cannot-be-executed» to
result in an error message without sending an email; however,
currently, bts(1) attempts to send the email (through /usr/sbin/sendmail).

> > for example, «chmod -x
> > ~/bin/mysendmail && bts --sendmail="$HOME/bin/mysendmail"» would
> > reproduce the bug.
> > 
> > As I said earlier, using the wrong sendmail binary can lead to an email
> > being sent using incorrect settings (e.g., using the wrong relayhost or
> > from address), which IMHO merits a greater-than-wishlist severity.
> 
> Yah  but if that happend due to wrong file permission, who do you blame?
> I am a bit intrigued. ...
> 

It is my fault that the argument to the --sendmail option was invalid.
This bug report is asking to change bts(1)'s handling of invalid values.

> > A separate issue is how the argument to the --sendmail option should be
> > handled.  There are several sensible options.¹  Currently, the code
> > validates the argument, splits it on whitespace, and passes it to
> > execve().  As you say, asking to add a mode whereby the argument would
> > be passed to system() instead would be a wishlist item.  However, I did
> > not request that; I simply asked to change bts(1)'s behaviour when the
> > validation fails.
> 
> Are you asking better error message?  Aha, ... OK.  But that is wishlist
> though.
> 

No, I'm not asking for a different error message.  I am asking for
a behaviour change.

The current behaviour is: «bts --sendmail="$value"» may execute
/usr/sbin/sendmail instead, if "$value" fails some validation.

The behaviour I am asking for is: «bts --sendmail="$value"» either uses
"$value" as the sendmail command, or prints an error message.

I am asking to eliminate the case where I specify --sendmail="$value"
but *another* sendmail is used.

> > I suggested that either the validation should not be
> > done in the first place (just call execve() and let it error out), or
> > a failed validation should result in a die() rather than in proceeding
> > with a different sendmail binary than specified by the user.
> 
> OK.  I see.
> 
> > Perhaps the "consider interpreting the argument to the --sendmail option
> > using Text::ParseWords(3perl) or system(3)" discussion should be spun off
> > into a separate bug report?  After all, it is mostly orthogonal to the
> > "Change the behaviour upon validation failure" issue.
> 
> Thanks,
> 
> Osamu

Is the request clearer now?  If not, I'd be happy to try and clarify it
further...

Cheers,

Daniel