On Mon, Jan 27, 2020 at 5:17 AM Zeger-Jan van de Weg <git@xxxxxxxxxxxxx> wrote:
> When setting values in the git config, the value is part of the
> arguments for execution. This potentially leaks the value through
> logging, or other programs like `ps`.
>
> Add the `--stdin` option that reads from stdin for instructions to set
> and unset values to hide them from prying eyes. The instructions are based
> on the `update-ref` DSL, and accept the set and unset commands.
>
> Signed-off-by: Zeger-Jan van de Weg <git@xxxxxxxxxxxxx>
> ---
> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> @@ -259,6 +264,30 @@ Valid `<type>`'s include:
> +STDIN
> +-----
> +
> +With `--stdin`, config reads instructions from standard input and performs
> +all modifications in sequence.
> +
> +Specify commands of the form:
> +
> + set SP <key> SP <newvalue>
> + unset SP <key>
If you follow the precedent of the git-update-ref documentation, these
should be:
set SP <key> SP <newvalue> LF
unset SP <key> LF
I'm not sure we really need to be calling the value "newvalue" (I
guess you picked that up from git-update-ref.txt). "value" should be
fine, thus:
set SP <key> SP <value> LF
unset SP <key> LF
> +Alternatively, use `-z` or `--null` to specify in NUL-terminated format, without
> +quoting:
> +
> + set SP <key> NULL <newvalue>
> + unset SP <key>
A few comments:
First, this is talking about the NUL character, not a NULL pointer,
so: s/NULL/NUL/
Second, this doesn't give any indication about how the lines should be
terminated. It should instead be written as:
set SP <key> NUL <value> NUL
unset SP <key> NUL
Third, importantly, unlike git-update-ref from which this DSL takes
inspiration and in which "refs" might have oddball names for which
NUL-termination makes sense, it's hard to imagine a case in which a
configuration key would be so strange as to warrant NUL-termination.
This observation suggests a simpler DSL in which only <value> is
NUL-terminated:
set SP <key> SP <value> NUL
unset SP <key> LF
(The proposed code changes in config.c would need adjustment, as well,
to implement this revised DSL.)
> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> @@ -380,6 +380,66 @@ test_expect_success '--add' '
> +test_expect_success '--stdin works on no input' '
> + echo -n "" | git config --stdin
> +'
`echo -n` is not portable. If you want no input at all, either grab it
from /dev/null:
git config --stdin </dev/null
or use `printf` to suppress printing of the line-terminator:
printf "" | git config --stdin
> +test_expect_success '--stdin with --null flag' '
> + echo -ne "set bar.baz\0false" | git config --stdin --null &&
> + Git config --get bar.baz >output &&
> + echo false >expect &&
> + test_cmp expect output
> +'
Likewise, non-portable use of `echo -n` and `echo "...\0...". Use
`printf` instead:
printf "set bar.baz\0false" | git config --stdin --null &&
(But note that this isn't even testing NUL-termination of <value>.)
