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

[PATCH] stash: be careful what we store

From
Junio C Hamano <gitster@pobox.com>
Date
Oct 11, 2023, 20:44 UTC
Message-ID
<xmqqbkd4lwj0.fsf_-_@gitster.g>
In-Reply-To
<xmqqzg0pnmv5.fsf@gitster.g>

"git stash store" is meant to store what "git stash create" produces, as these two are implementation details of the end-user facing "git stash save" command. Even though it is clearly documented as such, users would try silly things like "git stash store HEAD" to render their stash unusable.

Worse yet, because "git stash drop" does not allow such a stash entry to be removed, "git stash clear" would be the only way to recover from such a mishap. Reuse the logic that allows "drop" to refrain from working on such a stash entry to teach "store" to avoid storing an object that is not a stash entry in the first place.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
  > It may be just the matter of doing something silly like this.
  > Not even compile tested, but hopefully it is sufficient to
  > convey the idea.
  Now it is at least compile-tested.
 builtin/stash.c  | 6 ++++++
 t/t3903-stash.sh | 4 ++++
 2 files changed, 10 insertions(+)
diff --git a/builtin/stash.c b/builtin/stash.c
index 3a4f9fd566..8073ef4019 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -977,6 +977,12 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 static int do_store_stash(const struct object_id *w_commit, const char *stash_msg,
 			  int quiet)
 {
+	struct stash_info info;
+	char revision[GIT_MAX_HEXSZ];
+
+	oid_to_hex_r(revision, w_commit);
+	assert_stash_like(&info, revision);
+
 	if (!stash_msg)
 		stash_msg = "Created via \"git stash store\".";
 
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 376cc8f4ab..35c8569aea 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -931,6 +931,10 @@ test_expect_success 'store called with invalid commit' '
 	test_must_fail git stash store foo
 '
 
+test_expect_success 'store called with non-stash commit' '
+	test_must_fail git stash store HEAD
+'
+
 test_expect_success 'store updates stash ref and reflog' '
 	git stash clear &&
 	git reset --hard &&
-- 
2.42.0-345-gaab89be2eb
Previous: Erik Cervin Edin
Message 4 of 4 in “Bug: git stash store can create stash entries that can't be dropped”
  1. Erik Cervin EdinOct 11, 2023
  2. Junio C HamanoOct 11, 2023
  3. Erik Cervin EdinOct 11, 2023
  4. stash: be careful what we storeJunio C Hamano, Oct 11, 2023

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.