git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
redoste <redoste@redoste.xyz>
Date
Jul 6, 2025, 17:34 UTC
Message-ID
<20250706173450.12995-1-redoste@redoste.xyz>
In-Reply-To
<20250704230829.29696-1-redoste@redoste.xyz>

Detaching the filename string from the tempfile structure used to cause delete_tempfile() to fail and the temporary file was not cleaned up.

While it's possible to get rid of the allocation and copy from xstrdup(), it keeps the code symetric with the other branch since interpolate_path() also allocates and ssh_signing_key_file is freed in both cases.

Helped-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: redoste <redoste@redoste.xyz>
---
v1->v2:
 * add a test case that checks for temporary files after signing commits
 * add small explaination about the use of xstrdup() in the commit
   body
 gpg-interface.c              |  2 +-
 t/t7528-signed-commit-ssh.sh | 16 ++++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/gpg-interface.c b/gpg-interface.c
index 0896458de5..bdcc8c2a2e 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -1048,7 +1048,7 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
 				    key_file->filename.buf);
 			goto out;
 		}
-		ssh_signing_key_file = strbuf_detach(&key_file->filename, NULL);
+		ssh_signing_key_file = xstrdup(key_file->filename.buf);
 	} else {
 		/* We assume a file */
 		ssh_signing_key_file = interpolate_path(signing_key, 1);
diff --git a/t/t7528-signed-commit-ssh.sh b/t/t7528-signed-commit-ssh.sh
index 065f780636..1a8d96f355 100755
--- a/t/t7528-signed-commit-ssh.sh
+++ b/t/t7528-signed-commit-ssh.sh
@@ -390,6 +390,22 @@ test_expect_success GPGSSH 'check config gpg.format values' '
 	test_must_fail git commit -S --amend -m "fail"
 '
 
+test_expect_success GPGSSH 'check temporary files clean up when signing commits' '
+	test_config gpg.format ssh &&
+	eval $(ssh-agent) &&
+	test_when_finished "kill ${SSH_AGENT_PID}" &&
+	mkdir tmpdir &&
+	TMPDIR="$(pwd)/tmpdir" &&
+	export TMPDIR &&
+	ssh-add "${GPGSSH_KEY_PRIMARY}" &&
+	echo 1 >file && git add file &&
+	git commit -a -m inline -S"$(cat "${GPGSSH_KEY_PRIMARY}.pub")" &&
+	echo 2 >file &&
+	git commit -a -m file -S"${GPGSSH_KEY_PRIMARY}" &&
+	find tmpdir -type f >tmpfiles &&
+	test_line_count = 0 tmpfiles
+'
+
 test_expect_failure GPGSSH 'detect fudged commit with double signature (TODO)' '
 	sed -e "/gpgsig/,/END PGP/d" forged1 >double-base &&
 	sed -n -e "/gpgsig/,/END PGP/p" forged1 | \
-- 
2.49.0
Previous: redosteNext: brian m. carlson
Message 10 of 21 in “ssh signing: don't detach the filename strbuf from key_file tempfile”
  1. ssh signing: don't detach the filename strbuf from key_file tempfileredoste, Jul 4, 2025
  2. Jeff KingJul 5, 2025
  3. brian m. carlsonJul 5, 2025
  4. redosteJul 5, 2025
  5. redosteJul 5, 2025
  6. brian m. carlsonJul 6, 2025
  7. Jeff KingJul 6, 2025
  8. redosteJul 6, 2025
  9. redosteJul 6, 2025
  10. ssh signing: don't detach the filename strbuf from key_file tempfileredoste, Jul 6, 2025
  11. brian m. carlsonJul 6, 2025
  12. Patrick SteinhardtJul 7, 2025
  13. Phillip WoodJul 7, 2025
  14. redosteJul 7, 2025
  15. Phillip WoodJul 7, 2025
  16. redosteJul 7, 2025
  17. Phillip WoodJul 7, 2025
  18. ssh signing: don't detach the filename strbuf from key_file tempfileredoste, Jul 7, 2025
  19. Junio C HamanoJul 7, 2025
  20. Patrick SteinhardtJul 8, 2025
  21. Phillip WoodJul 8, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.