patchette

fix: STACK base and tip must be full commit IDs

commit 9cd6edf32ec8c7150335c2019a246c1982313237 · parent 46343bf21d · browse this tree
Ximianth, 2026-10-21 09:12:03 UTC

parse_stack took base and tip verbatim.  Anything git could resolve got
through: `base main` reached replay and died inside hash-object, with
the error pinned on the first patch file rather than the STACK; an
abbreviated tip was reported as a MISMATCH against a replay that was
in fact correct; and `tip ` with no value parsed as an absent tip, so
verify passed without cross-checking anything.

FORMAT.md now states the rule: full lowercase hex, 40 characters for
SHA-1 or 64 for SHA-256.  Abbreviations and ref names are refused
because they can resolve differently from repo to repo, or over time.
parse_stack checks each value as it reads it, so the error names the
STACK and the offending value.

The three new fail-closed cases each failed before this change.  The
64-character form isn't covered by the suite (there's no SHA-256
corpus); a SHA-256 export and verify was checked by hand.

diff --git a/bin/patchette b/bin/patchette
index 5355755..cd4762e 100755
--- a/bin/patchette
+++ b/bin/patchette
@@ -91,6 +91,7 @@ absdir() {
 ## - @state STACK_REF, STACK_BASE, STACK_TIP, STACK_SERIES[] -- the parsed fields
 parse_stack() {
 	local f=$1 line mode=head
+	local oid='^[0-9a-f]{40}([0-9a-f]{24})?$' # SHA-1 or SHA-256, full and lowercase
 	declare -A seen=()
 	STACK_REF='' STACK_BASE='' STACK_TIP='' STACK_SERIES=()
 	while IFS= read -r line; do
@@ -103,8 +104,14 @@ parse_stack() {
 		fi
 		case $line in
 		"ref "*) STACK_REF=${line#ref } ;;
-		"base "*) STACK_BASE=${line#base } ;;
-		"tip "*) STACK_TIP=${line#tip } ;;
+		"base "*)
+			STACK_BASE=${line#base }
+			[[ $STACK_BASE =~ $oid ]] || die "$f: base is not a full commit ID: $STACK_BASE"
+			;;
+		"tip "*)
+			STACK_TIP=${line#tip }
+			[[ $STACK_TIP =~ $oid ]] || die "$f: tip is not a full commit ID: $STACK_TIP"
+			;;
 		series) mode=series ;;
 		*) die "$f: unrecognized line: $line" ;;
 		esac
diff --git a/docs/hacking/FORMAT.md b/docs/hacking/FORMAT.md
index 96950bd..99d14dc 100644
--- a/docs/hacking/FORMAT.md
+++ b/docs/hacking/FORMAT.md
@@ -110,6 +110,10 @@ Of the key value entries:
   It may also be disregarded in some workflows: e.g., when hand-editing patch files,
   there's no expectable value for tip until the branch has been built.
 
+The `base` and `tip` values are full commit IDs in lowercase hex:
+40 characters in a SHA-1 repo, 64 in a SHA-256 one.
+Abbreviations and ref names are errors (they can resolve differently from repo to repo, or over time).
+
 In the `series` section:
 
 - each line is a patch name.  These match `^[a-z0-9-]+$`,
diff --git a/tests/test-failclosed.sh b/tests/test-failclosed.sh
index 6c77dd5..92754d6 100755
--- a/tests/test-failclosed.sh
+++ b/tests/test-failclosed.sh
@@ -71,6 +71,21 @@ sed -i '1s/.*/parents no-such-slug/' "$TAMPER/$fancy/add-sprocket-docs.patch"
 EXPECT refusal "an unknown parent reference is an error" "not BASE or an earlier slug" -- \
 	"$PATCHETTE" verify "$work/corpus/dev" "$TAMPER"
 
+# base and tip must be full commit IDs: a ref name or an abbreviation could
+# resolve differently in another repo, or later, so neither is guessed at.
+clone_layout refbase
+sed -i 's/^base .*/base main/' "$TAMPER/$fancy/STACK"
+EXPECT refusal "a ref name as base is an error" "base is not a full commit ID" -- \
+	"$PATCHETTE" verify "$work/corpus/dev" "$TAMPER"
+clone_layout shorttip
+sed -i 's/^tip \(.\{12\}\).*/tip \1/' "$TAMPER/$fancy/STACK"
+EXPECT refusal "an abbreviated tip is an error" "tip is not a full commit ID" -- \
+	"$PATCHETTE" verify "$work/corpus/dev" "$TAMPER"
+clone_layout emptytip
+sed -i 's/^tip .*/tip /' "$TAMPER/$fancy/STACK"
+EXPECT refusal "a tip line with no value is an error, not an absent tip" "tip is not a full commit ID" -- \
+	"$PATCHETTE" verify "$work/corpus/dev" "$TAMPER"
+
 # A duplicate series entry is an error.
 clone_layout dupseries
 echo "note-to-self" >>"$TAMPER/$fancy/STACK"