Re: [PATCH] ssh signing: don't detach the filename strbuf from key_file tempfile

From: redoste <redoste@redoste.xyz>
Date: Jul 5, 2025, 22:50 UTC
Message-ID: <DB4HMD2HLG24.1V8Y9JDW6BACQ@redoste.xyz>
In-Reply-To: <aGmGCmkwC1HlSyog@fruit.crustytoothpaste.net>

On Sat Jul 5, 2025 at 22:07 CEST, brian m. carlson wrote:

Show 8 quoted lines

> On 2025-07-05 at 19:21:13, Jeff King wrote:
>> I wonder if it is worth adding a test, or if it would be too weirdly
>> focused on this obscure case to be very useful against future
>> regressions.
>
> I don't have a strong view either way, but I do wonder if it's a good
> idea to have the testsuite poking around in `/tmp`, although maybe if we
> honour `TMPDIR` then it would be possible to do in a tidy way.

I looked into adding a test, but I didn't find any other tests checking for temporary files and I agree that messing in /tmp doesn't feel really appropriate for the testsuite.

Maybe something like this?

diff --git a/t/t7528-signed-commit-ssh.sh b/t/t7528-signed-commit-ssh.sh
index 065f780636..359dc8eba8 100755
--- a/t/t7528-signed-commit-ssh.sh
+++ b/t/t7528-signed-commit-ssh.sh
@@ -85,6 +85,7 @@ test_expect_success GPGSSH 'sign commits using literal public keys with ssh-agen
	eval $(ssh-agent) &&
	test_when_finished "kill ${SSH_AGENT_PID}" &&
	ssh-add "${GPGSSH_KEY_PRIMARY}" &&
+	export TMPDIR=$(pwd) &&
	echo 1 >file && git add file &&
	git commit -a -m rsa-inline -S"$(cat "${GPGSSH_KEY_PRIMARY}.pub")" &&
	echo 2 >file &&
@@ -95,7 +96,8 @@ test_expect_success GPGSSH 'sign commits using literal public keys with ssh-agen
	git commit -a -m ecdsa-inline -S"key::$(cat "${GPGSSH_KEY_ECDSA}.pub")" &&
	echo 4 >file &&
	test_config user.signingkey "key::$(cat "${GPGSSH_KEY_ECDSA}.pub")" &&
-	git commit -a -m ecdsa-config -S
+	git commit -a -m ecdsa-config -S &&
+	! ls .git_signing_key_tmp*
 '
 
 test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'create signed commits with keys having defined lifetimes' '

I can add it in a v2 if you think it's a good way to test it.

> I also have some friends who are trans and have transitioned or are in
> the process of transitioning but have simply not gotten around to
> getting legal paperwork done[1].
This is the exact reason why I'm not very comfortable with using my
legal or real name, (well, it's mostly because I still can't find a name
I like).
And since it's a simple patch that's probably not even copyrightable, I
figured out that using a pseudonym was fine.

Since I knew that the Linux kernel changed their documentation to remove
the use of "real name", I thought it was more common and didn't relly
think about it a lot.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d4563201f33a022fc0353033d9dfeb1606a88330

I'm sorry, I should have read the git documentation more thoroughly.

If it's really an issue I don't mind signing off with a different and
more distinctive name.

-- 
redoste