Re: [PATCH v4 6/7] ssh signing: make fmt-merge-msg consider key lifetime

From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Fabian Stelzer <fs@gigacodes.de>
Cc: git@vger.kernel.org, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>
Subject: Re: [PATCH v4 6/7] ssh signing: make fmt-merge-msg consider key lifetime
Date: Sun, 5 Dec 2021 20:23:44 +0100	[thread overview]
Message-ID: <20211205192344.GB624717@szeder.dev> (raw)
In-Reply-To: <20211130141112.78193-7-fs@gigacodes.de>

On Tue, Nov 30, 2021 at 03:11:11PM +0100, Fabian Stelzer wrote:
> diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh
> index 06c5fb5615..2dd2423643 100755
> --- a/t/t6200-fmt-merge-msg.sh
> +++ b/t/t6200-fmt-merge-msg.sh
> @@ -91,6 +91,26 @@ test_expect_success GPGSSH 'created ssh signed commit and tag' '
>  	git tag -s -u"${GPGSSH_KEY_UNTRUSTED}" -m signed-ssh-tag-msg-untrusted signed-untrusted-ssh-tag left
>  '
>  
> +test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'create signed tags with keys having defined lifetimes' '
> +	test_when_finished "test_unconfig commit.gpgsign" &&
> +	test_config gpg.format ssh &&
> +	git checkout -b signed-expiry-ssh &&
> +	touch file &&
> +	git add file &&
> +
> +	echo expired >file && test_tick && git commit -a -m expired -S"${GPGSSH_KEY_EXPIRED}" &&
> +	git tag -s -u "${GPGSSH_KEY_EXPIRED}" -m expired-signed expired-signed &&
> +
> +	echo notyetvalid >file && test_tick && git commit -a -m notyetvalid -S"${GPGSSH_KEY_NOTYETVALID}" &&
> +	git tag -s -u "${GPGSSH_KEY_NOTYETVALID}" -m notyetvalid-signed notyetvalid-signed &&
> +
> +	echo timeboxedvalid >file && test_tick && git commit -a -m timeboxedvalid -S"${GPGSSH_KEY_TIMEBOXEDVALID}" &&
> +	git tag -s -u "${GPGSSH_KEY_TIMEBOXEDVALID}" -m timeboxedvalid-signed timeboxedvalid-signed &&
> +
> +	echo timeboxedinvalid >file && test_tick && git commit -a -m timeboxedinvalid -S"${GPGSSH_KEY_TIMEBOXEDINVALID}" &&
> +	git tag -s -u "${GPGSSH_KEY_TIMEBOXEDINVALID}" -m timeboxedinvalid-signed timeboxedinvalid-signed
> +'
> +
>  test_expect_success 'message for merging local branch' '
>  	echo "Merge branch ${apos}left${apos}" >expected &&
>  
> @@ -137,6 +157,40 @@ test_expect_success GPGSSH 'message for merging local tag signed by unknown ssh
>  	! grep "${GPGSSH_BAD_SIGNATURE}" actual &&
>  	grep "${GPGSSH_KEY_NOT_TRUSTED}" actual
>  '
> +
> +test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag signed by expired ssh key' '
> +	test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
> +	git checkout main &&
> +	git fetch . expired-signed &&
> +	git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
> +	! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual

Are these messages supposed to go to standard output or error?  If
it's the former, then please drop the unnecessary (and confusing)
'2>&1' redirection, but if it's the latter, then save and 'grep' only
stderr.

> +'
> +
> +test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag signed by not yet valid ssh key' '
> +	test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
> +	git checkout main &&
> +	git fetch . notyetvalid-signed &&
> +	git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
> +	! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
> +'
> +
> +test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag signed by valid timeboxed ssh key' '
> +	test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
> +	git checkout main &&
> +	git fetch . timeboxedvalid-signed &&
> +	git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
> +	grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual &&
> +	! grep "${GPGSSH_BAD_SIGNATURE}" actual
> +'
> +
> +test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag signed by invalid timeboxed ssh key' '
> +	test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
> +	git checkout main &&
> +	git fetch . timeboxedinvalid-signed &&
> +	git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
> +	! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
> +'
> +
>  test_expect_success 'message for merging external branch' '
>  	echo "Merge branch ${apos}left${apos} of $(pwd)" >expected &&
>  
> -- 
> 2.31.1
> 