Subject: Re: [PATCH v4 3/3] send-email: add option to generate passswords like OAuth2 tokens

From: Junio C Hamano <[email-masked]>
Date: Wed, 23 Apr 2025 12:03:50 -0700

Aditya Garg <[email-masked]> writes:

> @@ -230,6 +230,14 @@ or on the command line. If a username has been specified (with
>  specified (with `--smtp-pass` or `sendemail.smtpPass`), then
>  a password is obtained using 'git-credential'.
>  
> +--smtp-passeval[=<command>]::

Lose the pair of [] that marks the value optional.  Compare it with,
say, --smtp-user that is described as:

    --smtp-user=<user>::
            Username for SMTP-AUTH. Default is ...

because they are defined in %options (below) in a similar way, like
so:

>  		    "smtp-user=s" => \$smtp_authuser,
>  		    "smtp-pass:s" => \$smtp_authpass,
> +		    "smtp-passeval=s" => \$smtp_authpasseval,
>  		    "smtp-ssl" => sub { $smtp_encryption = 'ssl' },

taking a string value =s that is not optional.

> +	Generate password like OAuth2 token for SMTP AUTH. If specified,
> +	it will use the output of the command specified as a password for
> +	authentication.
> ++

> diff --git a/git-send-email.perl b/git-send-email.perl
> index a18e978e22..cafb9aa43b 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -59,6 +59,8 @@ sub usage {
>      --smtp-server-port      <int>  * Outgoing SMTP server port.
>      --smtp-user             <str>  * Username for SMTP-AUTH.
>      --smtp-pass             <str>  * Password for SMTP-AUTH; not necessary.
> +    --smtp-passeval         <str>  * Path to script or a command to generate
> +                                     password like OAuth2 token for SMTP-AUTH.
>      --smtp-encryption       <str>  * tls or ssl; anything else disables.
>      --smtp-ssl                     * Deprecated. Use '--smtp-encryption ssl'.
>      --smtp-ssl-cert-path    <str>  * Path to ca-certificates (either directory or file).

Looking good.

> +	# If smtpPassEval is set, run the user specified command to get the password
> +	if (defined $smtp_authpasseval) {
> +		printf __("Executing token generating script: %s\n"), $smtp_authpasseval;
> +		chomp(my $generated_password = `$smtp_authpasseval 2>&1`);

How careful do we need to protect ourselves against a bad value in
this variable (like "rm -rf $HOME; password-command") ?  Are we OK
with trusting that the command line and the configuration file are
not under control of an attacker?  I am assuming it is OK, but you
folks have thought about this code path much longer than I have, so
I thought I should ask just to make sure.

Thanks.