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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 23, 2025, 19:03 UTC
Message-ID: <xmqqwmbaya21.fsf@gitster.g>
In-Reply-To: <PN3PR01MB9597B50EF69AD097C594F844B8BA2@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM>

Aditya Garg <gargaditya08@live.com> writes:

Show 5 quoted lines

> @@ -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.
> ++

Show 13 quoted lines

> 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.