hack: repo-publish: refuse to commit over a racing publish

cmd/hardhat/repoctl.go

@@ -237,6 +237,12 @@ var repoctlRefreshCommand = withTxExpireAfterFlag(&cli.Command{
 var repoctlSnapshotCommand = withTxExpireAfterFlag(&cli.Command{
 	Name:  "snapshot",
 	Usage: "update the repo's snapshot.json",
+	Flags: []cli.Flag{
+		&cli.Int64Flag{
+			Name:  "if-timestamp-version",
+			Usage: "fail unless the repository is still at this timestamp version (0: has no timestamp yet), i.e. the one the targets were computed from",
+		},
+	},
 	Arguments: []cli.Argument{
 		&cli.StringArgs{
 			Name:      "targets",
@@ -255,6 +261,12 @@ var repoctlSnapshotCommand = withTxExpireAfterFlag(&cli.Command{
 		}
 		tx.RefTime, _ = ctxext.RefTime(ctx)
 
+		if cmd.IsSet("if-timestamp-version") {
+			if err := tx.Apply(ctx, tufrepo.RequireTimestampVersion(cmd.Int64("if-timestamp-version"))); err != nil {
+				return err
+			}
+		}
+
 		if err := applyTxExpireAfter(ctx, tx); err != nil {
 			return err
 		}

hack/repo-publish.sh

@@ -245,12 +245,22 @@ read -ra builder_keyids <<<"$(jq -rM '.signed.roles.targets.keyids[]' <"$root_js
 builder_sign_flags=("${builder_keyids[@]/#/--keyid=}")
 hardhat_targets_flags+=("${builder_sign_flags[@]}")
 
+# The repository state everything below is computed from. Another publisher may
+# commit before we do; snapshot then refuses to commit instead of silently
+# dropping what they published. Only timestamp.json is rewritten in place, so
+# read it just once; the versioned files it leads to never change.
+base_timestamp_version=0
+if [ -f "$REPO_DIR/timestamp.json" ]; then
+	base_timestamp_json="$(mktempfile timestamp.json)"
+	cp "$REPO_DIR/timestamp.json" "$base_timestamp_json"
+	base_timestamp_version="$(jq -rM '.signed.version' <"$base_timestamp_json")"
+fi
+
 if [ -n "$merge_old_targets" ]; then
 	# Figure out the latest targets.json.
-	latest_timestamp_json="$REPO_DIR/timestamp.json"
 	# If the repository was just initialised, we skip this.
-	if [ -f "$latest_timestamp_json" ]; then
-		latest_snapshot_json="$REPO_DIR/$(jq -rM '.signed.meta["snapshot.json"].version' <"$latest_timestamp_json").snapshot.json"
+	if [ "$base_timestamp_version" != 0 ]; then
+		latest_snapshot_json="$REPO_DIR/$(jq -rM '.signed.meta["snapshot.json"].version' <"$base_timestamp_json").snapshot.json"
 		latest_targets_json="$REPO_DIR/$(jq -rM '.signed.meta["targets.json"].version' <"$latest_snapshot_json").targets.json"
 		hardhat_targets_flags+=("--include-from=$latest_targets_json")
 	else
@@ -310,4 +320,5 @@ PUBLISHER_KEYSTORE="$KEY_DIR/quarryd-keys"
 hardhat repoctl snapshot \
 	--keystore="$PUBLISHER_KEYSTORE" \
 	--repo-metadir="$REPO_DIR" \
+	--if-timestamp-version="$base_timestamp_version" \
 	targets="$targets_json"

internal/tufrepo/tx.go

@@ -554,6 +554,33 @@ func InitTxn(initRoot *tufext.SignedRoot) *Transaction {
 // since [Repository.TxnStart].
 var ErrClobberedTransaction = errors.New("transaction rejected because repository state has changed")
 
+// RequireTimestampVersion returns a [TxnOp] that fails with
+// [ErrClobberedTransaction] unless the transaction started from the timestamp
+// with the given version, or from a repository without a timestamp if version
+// is 0.
+//
+// [Repository.TxnCommit] only detects changes made after [Repository.TxnStart].
+// A caller which computed the new role data from an earlier read of the
+// repository (such as merging the previous targets) uses this to extend that
+// check back to its read: timestamp versions are never reused, as
+// [Repository.TxnCommit] writes each one without clobbering.
+func RequireTimestampVersion(version int64) TxnOp {
+	return NewTxnOp(
+		fmt.Sprintf("require timestamp version %d", version),
+		func(_ context.Context, tx *Transaction) error {
+			var current int64
+			if tx.timestamp != nil {
+				current = tx.timestamp.Signed.Version
+			}
+			if current != version {
+				return fmt.Errorf("%w: expected timestamp version %d, found %d",
+					ErrClobberedTransaction, version, current)
+			}
+			return nil
+		},
+	)
+}
+
 // ErrInvalidTransactionState is returned from [Repository.TxnCommit] if a
 // transaction has an internally invalid state that means it cannot committed
 // to a repository.

internal/tufrepo/tx_test.go

@@ -924,3 +924,58 @@ func TestTxnCommit_Twice_DoesNotDestroyFirstCommit(t *testing.T) {
 	assert.Equal(t, firstSnap.Version, liveSnap.Version)
 	assert.Equal(t, firstSnap.Hashes, liveSnap.Hashes)
 }
+
+// ----- RequireTimestampVersion -------------------------------------------
+
+// commitTarget publishes one target in its own transaction, as another
+// publisher would.
+func commitTarget(ctx context.Context, t *testing.T, bs *bootstrap, path string) *tufext.SignedTimestamp {
+	t.Helper()
+	tx, err := bs.repo.TxnStart(ctx)
+	require.NoError(t, err)
+	require.NoError(t, tx.Apply(ctx, addTargetOp(path, 1)))
+	_, err = tx.Sign(ctx, bs.store)
+	require.NoError(t, err)
+	ts, err := bs.repo.TxnCommit(ctx, tx)
+	require.NoError(t, err)
+	return ts
+}
+
+func TestRequireTimestampVersion_Current(t *testing.T) {
+	ctx := context.Background()
+	bs := bootstrapRepo(t)
+	base := currentTimestamp(ctx, t, bs.repo).Signed.Version
+
+	tx, err := bs.repo.TxnStart(ctx)
+	require.NoError(t, err)
+	require.NoError(t, tx.Apply(ctx, tufrepo.RequireTimestampVersion(base)))
+}
+
+// A publisher computes its new targets from the repository it read, and only
+// starts the transaction afterwards. Another publisher committing in between is
+// invisible to TxnCommit's check, so the stale base has to be caught here, or
+// the other publisher's targets are silently dropped.
+func TestRequireTimestampVersion_ChangedBeforeTxnStart(t *testing.T) {
+	ctx := context.Background()
+	bs := bootstrapRepo(t)
+	base := currentTimestamp(ctx, t, bs.repo).Signed.Version
+
+	racing := commitTarget(ctx, t, bs, "t/racing.bin")
+	require.NotEqual(t, base, racing.Signed.Version)
+
+	tx, err := bs.repo.TxnStart(ctx)
+	require.NoError(t, err)
+	err = tx.Apply(ctx, tufrepo.RequireTimestampVersion(base))
+	require.ErrorIs(t, err, tufrepo.ErrClobberedTransaction)
+	assert.ErrorContains(t, err, fmt.Sprintf("expected timestamp version %d, found %d", base, racing.Signed.Version))
+}
+
+func TestRequireTimestampVersion_ZeroRejectsExistingTimestamp(t *testing.T) {
+	ctx := context.Background()
+	bs := bootstrapRepo(t)
+
+	tx, err := bs.repo.TxnStart(ctx)
+	require.NoError(t, err)
+	err = tx.Apply(ctx, tufrepo.RequireTimestampVersion(0))
+	require.ErrorIs(t, err, tufrepo.ErrClobberedTransaction)
+}