From c2d09ab05b3030b01443095c82eb27b66b44f4e8 Mon Sep 17 00:00:00 2001
From: Astrid Gealer <astrid@gealer.email>
Date: Thu, 22 Oct 2026 02:22:20 +0100
Subject: [PATCH 1/3] sec(config): add a token-pinned atomic cancel for the
 email-link cancel

CancelExecutionAtomicTokenPinned is CancelExecutionAtomic with the approval
token checked in the same UPDATE as the status change: the stored hash must
still match the caller's token and be unexpired. A token rotated by a
re-notification after the caller's snapshot check now loses the cancel with
ErrApprovalTokenChanged instead of canceling the row (issue #1190), the
cancel-side twin of the #590 token-pinned transition.

Real-PostgreSQL tests cover the pinned cancel's outcomes, a pinned action
blocked on the row lock behind a committed rotation, and an unsynchronized
rotation race for both the pinned cancel and the pinned claim to running.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 internal/analytics/collector_test.go          |   4 +
 internal/config/interfaces.go                 |   8 +
 internal/config/store_postgres.go             |  51 +++-
 .../config/store_postgres_pgxmock_test.go     |   5 +-
 ...store_postgres_token_pin_cancel_db_test.go | 248 ++++++++++++++++++
 internal/mocks/stores.go                      |  11 +
 internal/server/test_helpers_test.go          |   4 +
 7 files changed, 323 insertions(+), 8 deletions(-)
 create mode 100644 internal/config/store_postgres_token_pin_cancel_db_test.go

diff --git a/internal/analytics/collector_test.go b/internal/analytics/collector_test.go
index 2400b1ad9..908e12fe3 100644
--- a/internal/analytics/collector_test.go
+++ b/internal/analytics/collector_test.go
@@ -347,6 +347,10 @@ func (m *mockConfigStore) CancelExecutionAtomic(ctx context.Context, tx pgx.Tx,
 	return false, "", nil
 }
 
+func (m *mockConfigStore) CancelExecutionAtomicTokenPinned(ctx context.Context, tx pgx.Tx, executionID string, cancelledBy *string, rawToken string) (bool, string, error) {
+	return false, "", nil
+}
+
 func (m *mockConfigStore) CancelScheduledExecutionAtomic(ctx context.Context, tx pgx.Tx, executionID string, cancelledBy *string) (bool, string, error) {
 	return false, "", nil
 }
diff --git a/internal/config/interfaces.go b/internal/config/interfaces.go
index f2e627890..4db3004e4 100644
--- a/internal/config/interfaces.go
+++ b/internal/config/interfaces.go
@@ -291,6 +291,14 @@ type StoreInterface interface {
 	// inside a WithTx block so the suppression cleanup and the status flip
 	// commit atomically.
 	CancelExecutionAtomic(ctx context.Context, tx pgx.Tx, executionID string, cancelledBy *string) (canceled bool, currentStatus string, err error)
+	// CancelExecutionAtomicTokenPinned is CancelExecutionAtomic that also
+	// requires the row's stored approval token hash to match rawToken and to
+	// be unexpired, in the same statement as the status change, so a token
+	// rotated or expired after the email-link cancel validated it cannot
+	// cancel the row (issue #1190). A lost token pin on a row still
+	// pending/notified returns an error wrapping ErrApprovalTokenChanged; a
+	// lost status CAS returns (false, currentStatus, nil) as usual.
+	CancelExecutionAtomicTokenPinned(ctx context.Context, tx pgx.Tx, executionID string, cancelledBy *string, rawToken string) (canceled bool, currentStatus string, err error)
 	// CancelScheduledExecutionAtomic atomically flips status from 'scheduled' to
 	// 'canceled' (canonical US spelling), setting canceled_by. Used by the
 	// Gmail-style pre-fire delay revoke path (issue #291 wave-2) to cancel a
diff --git a/internal/config/store_postgres.go b/internal/config/store_postgres.go
index c90c71ab5..6480a9cb7 100644
--- a/internal/config/store_postgres.go
+++ b/internal/config/store_postgres.go
@@ -1405,6 +1405,30 @@ func (s *PostgresStore) SetCancelledBy(ctx context.Context, executionID, cancell
 // status guard is inside the UPDATE rather than checked optimistically
 // before entering the tx.
 func (s *PostgresStore) CancelExecutionAtomic(ctx context.Context, tx pgx.Tx, executionID string, cancelledBy *string) (canceled bool, currentStatus string, err error) {
+	return s.cancelExecutionAtomic(ctx, tx, executionID, cancelledBy, nil)
+}
+
+// CancelExecutionAtomicTokenPinned is CancelExecutionAtomic that also requires
+// approval_token_hash to equal the hash of rawToken and the token to be
+// unexpired, checked in the same UPDATE as the status change. The email-link
+// cancel validated the token against a snapshot; a re-notification that
+// rotates the token between that read and this write would otherwise still
+// let the old, revoked token cancel the row (issue #1190, the cancel side of
+// #590). A lost token pin on a row still pending/notified returns an error
+// wrapping ErrApprovalTokenChanged; a lost status CAS returns (false,
+// currentStatus, nil) as CancelExecutionAtomic does. An empty rawToken never
+// matches.
+func (s *PostgresStore) CancelExecutionAtomicTokenPinned(ctx context.Context, tx pgx.Tx, executionID string, cancelledBy *string, rawToken string) (canceled bool, currentStatus string, err error) {
+	if rawToken == "" {
+		return false, "", fmt.Errorf("%w: execution %s: no approval token supplied", ErrApprovalTokenChanged, executionID)
+	}
+	tokenHash := HashApprovalToken(rawToken)
+	return s.cancelExecutionAtomic(ctx, tx, executionID, cancelledBy, &tokenHash)
+}
+
+// cancelExecutionAtomic is the shared body of CancelExecutionAtomic and
+// CancelExecutionAtomicTokenPinned; a nil tokenHash skips the token pin.
+func (s *PostgresStore) cancelExecutionAtomic(ctx context.Context, tx pgx.Tx, executionID string, cancelledBy, tokenHash *string) (canceled bool, currentStatus string, err error) {
 	// StatusCanceled is the canonical US-spelling value; see types.go.
 	q := `
 		UPDATE purchase_executions
@@ -1413,9 +1437,11 @@ func (s *PostgresStore) CancelExecutionAtomic(ctx context.Context, tx pgx.Tx, ex
 		       updated_at   = NOW()
 		 WHERE execution_id = $1
 		   AND status IN ('pending', 'notified')
+		   AND ($4::text IS NULL OR (approval_token_hash = $4
+		        AND (approval_token_expires_at IS NULL OR approval_token_expires_at > NOW())))
 		RETURNING status
 	`
-	rows, err := tx.Query(ctx, q, executionID, cancelledBy, StatusCanceled)
+	rows, err := tx.Query(ctx, q, executionID, cancelledBy, StatusCanceled, tokenHash)
 	if err != nil {
 		return false, "", fmt.Errorf("failed to cancel execution: %w", err)
 	}
@@ -1435,14 +1461,27 @@ func (s *PostgresStore) CancelExecutionAtomic(ctx context.Context, tx pgx.Tx, ex
 		return false, "", fmt.Errorf("failed to iterate cancel result: %w", rowsErr)
 	}
 
-	// Zero rows affected: execution either does not exist or has already
-	// transitioned out of pending/notified. Surface the current status so
-	// callers can return a meaningful 409 body.
+	// Zero rows affected: execution either does not exist, has already
+	// transitioned out of pending/notified, or (pinned) no longer carries the
+	// caller's token. Surface the current status so callers can return a
+	// meaningful 409 body.
+	currentStatus, err = s.cancelRejection(ctx, executionID, tokenHash != nil)
+	return false, currentStatus, err
+}
+
+// cancelRejection explains a zero-row cancelExecutionAtomic by probing the
+// row's current status. A row still pending/notified under a token pin lost
+// the pin, not the status race, so that case returns ErrApprovalTokenChanged.
+// Extracted to keep the caller under the gocyclo threshold.
+func (s *PostgresStore) cancelRejection(ctx context.Context, executionID string, tokenPinned bool) (currentStatus string, err error) {
 	existing, existErr := s.GetExecutionByID(ctx, executionID)
 	if existErr != nil {
-		return false, "", fmt.Errorf("execution not found or db error: %w", existErr)
+		return "", fmt.Errorf("execution not found or db error: %w", existErr)
 	}
-	return false, existing.Status, nil
+	if tokenPinned && (existing.Status == "pending" || existing.Status == "notified") {
+		return existing.Status, fmt.Errorf("%w: execution %s", ErrApprovalTokenChanged, executionID)
+	}
+	return existing.Status, nil
 }
 
 // CancelScheduledExecutionAtomic atomically transitions an execution from
diff --git a/internal/config/store_postgres_pgxmock_test.go b/internal/config/store_postgres_pgxmock_test.go
index 9f51b1b03..334435093 100644
--- a/internal/config/store_postgres_pgxmock_test.go
+++ b/internal/config/store_postgres_pgxmock_test.go
@@ -3037,12 +3037,13 @@ func TestPGXMock_CancelExecutionAtomic_WritesCanonicalStatus(t *testing.T) {
 
 	// The mock uses regexp matching. We pin on:
 	//   - "canceled_by" to confirm the new column is targeted (not cancelled_by)
-	//   - WithArgs: $1=execID, $2=&actor, $3=StatusCanceled
+	//   - WithArgs: $1=execID, $2=&actor, $3=StatusCanceled, $4=nil (no
+	//     token pin on the unpinned cancel, issue #1190)
 	// The RETURNING row also carries "canceled" so the caller's returned
 	// status is the canonical spelling end-to-end.
 	rows := pgxmock.NewRows([]string{"status"}).AddRow(StatusCanceled)
 	mock.ExpectQuery("canceled_by").
-		WithArgs("exec-1", &actor, StatusCanceled).
+		WithArgs("exec-1", &actor, StatusCanceled, (*string)(nil)).
 		WillReturnRows(rows)
 
 	// Pass mock directly as pgx.Tx (PgxPoolIface embeds pgx.Tx).
diff --git a/internal/config/store_postgres_token_pin_cancel_db_test.go b/internal/config/store_postgres_token_pin_cancel_db_test.go
new file mode 100644
index 000000000..aaf25cde5
--- /dev/null
+++ b/internal/config/store_postgres_token_pin_cancel_db_test.go
@@ -0,0 +1,248 @@
+//go:build integration
+// +build integration
+
+package config
+
+import (
+	"context"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/jackc/pgx/v5"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// Real PostgreSQL. Issue #1190: the email-link cancel and the immediate
+// email-link approve pin the approval token inside their status CAS, as the
+// delayed approve already did (issue #590), so a token rotated by a
+// re-notification between the caller's snapshot check and the CAS can no
+// longer move the row.
+
+const pinOldToken, pinNewToken = "pin-old-raw-token", "pin-new-raw-token"
+
+func tokenPinFixture(t *testing.T) (*PostgresStore, func(t *testing.T, expiresAt time.Time) *PurchaseExecution) {
+	t.Helper()
+	conn := setupTestContainerDB(t)
+	store := NewPostgresStore(conn)
+	ctx := context.Background()
+	plan := &PurchasePlan{Name: "token pin cancel", Enabled: true, Services: map[string]ServiceConfig{}, RampSchedule: PresetRampSchedules["immediate"]}
+	require.NoError(t, store.CreatePurchasePlan(ctx, plan))
+	newRow := func(t *testing.T, expiresAt time.Time) *PurchaseExecution {
+		t.Helper()
+		exec := &PurchaseExecution{PlanID: plan.ID, Status: "notified", StepNumber: 1, ScheduledDate: time.Now(),
+			ApprovalToken: HashApprovalToken(pinOldToken), ApprovalTokenExpiresAt: &expiresAt}
+		require.NoError(t, store.SavePurchaseExecution(ctx, exec))
+		return exec
+	}
+	return store, newRow
+}
+
+// pinnedCancel runs CancelExecutionAtomicTokenPinned in its own transaction,
+// as purchase.Manager.CancelExecution does.
+func pinnedCancel(ctx context.Context, store *PostgresStore, id, rawToken string) (canceled bool, status string, err error) {
+	err = store.WithTx(ctx, func(tx pgx.Tx) error {
+		var txErr error
+		canceled, status, txErr = store.CancelExecutionAtomicTokenPinned(ctx, tx, id, nil, rawToken)
+		return txErr
+	})
+	return canceled, status, err
+}
+
+// requireStillNotified asserts a refused action left the row notified and
+// holding rawToken.
+func requireStillNotified(ctx context.Context, t *testing.T, store *PostgresStore, id, rawToken string) {
+	t.Helper()
+	got, err := store.GetExecutionByID(ctx, id)
+	require.NoError(t, err)
+	assert.Equal(t, "notified", got.Status)
+	assert.True(t, ApprovalTokenMatches(got.ApprovalToken, rawToken), "stored token should still be %q", rawToken)
+}
+
+func TestPostgresStoreDB_CancelExecutionAtomicTokenPinned(t *testing.T) {
+	store, newRow := tokenPinFixture(t)
+	ctx := context.Background()
+
+	t.Run("token rotated after the snapshot loses the cancel", func(t *testing.T) {
+		exec := newRow(t, time.Now().Add(time.Hour))
+		rotated, err := store.RotatePendingApprovalToken(ctx, exec.ExecutionID, HashApprovalToken(pinNewToken), time.Now().Add(time.Hour))
+		require.NoError(t, err)
+		require.True(t, rotated)
+
+		canceled, _, err := pinnedCancel(ctx, store, exec.ExecutionID, pinOldToken)
+		require.ErrorIs(t, err, ErrApprovalTokenChanged)
+		assert.False(t, canceled)
+		requireStillNotified(ctx, t, store, exec.ExecutionID, pinNewToken)
+
+		canceled, status, err := pinnedCancel(ctx, store, exec.ExecutionID, pinNewToken)
+		require.NoError(t, err)
+		assert.True(t, canceled, "the current token still cancels")
+		assert.Equal(t, StatusCanceled, status)
+	})
+
+	t.Run("token expired after the snapshot loses the cancel", func(t *testing.T) {
+		exec := newRow(t, time.Now().Add(-time.Minute))
+		_, _, err := pinnedCancel(ctx, store, exec.ExecutionID, pinOldToken)
+		require.ErrorIs(t, err, ErrApprovalTokenChanged)
+		requireStillNotified(ctx, t, store, exec.ExecutionID, pinOldToken)
+	})
+
+	t.Run("wrong or empty token loses the cancel", func(t *testing.T) {
+		exec := newRow(t, time.Now().Add(time.Hour))
+		for _, tok := range []string{"not-the-token", ""} {
+			_, _, err := pinnedCancel(ctx, store, exec.ExecutionID, tok)
+			require.ErrorIs(t, err, ErrApprovalTokenChanged, "token %q", tok)
+			requireStillNotified(ctx, t, store, exec.ExecutionID, pinOldToken)
+		}
+	})
+
+	t.Run("matching token on a row already approved is a status CAS loss", func(t *testing.T) {
+		exec := newRow(t, time.Now().Add(time.Hour))
+		_, err := store.TransitionExecutionStatusTokenPinned(ctx, exec.ExecutionID, []string{"pending", "notified"}, "running", nil, pinOldToken)
+		require.NoError(t, err)
+		canceled, status, err := pinnedCancel(ctx, store, exec.ExecutionID, pinOldToken)
+		require.NoError(t, err, "a lost status race is reported through currentStatus, as on the unpinned cancel")
+		assert.False(t, canceled)
+		assert.Equal(t, "running", status)
+	})
+
+	t.Run("the unpinned cancel is unchanged", func(t *testing.T) {
+		exec := newRow(t, time.Now().Add(-time.Minute))
+		require.NoError(t, store.WithTx(ctx, func(tx pgx.Tx) error {
+			canceled, _, err := store.CancelExecutionAtomic(ctx, tx, exec.ExecutionID, nil)
+			assert.True(t, canceled, "the session cancel is not token-gated")
+			return err
+		}))
+	})
+}
+
+// waitForBlockedUpdate waits until some backend is waiting on a row lock, so
+// the test knows the second writer has reached the locked row.
+func waitForBlockedUpdate(ctx context.Context, t *testing.T, store *PostgresStore) {
+	t.Helper()
+	deadline := time.Now().Add(10 * time.Second)
+	for time.Now().Before(deadline) {
+		var waiting int
+		require.NoError(t, store.db.QueryRow(ctx, `SELECT count(*) FROM pg_stat_activity WHERE wait_event_type = 'Lock' AND datname = current_database()`).Scan(&waiting))
+		if waiting > 0 {
+			return
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	t.Fatal("the second writer never blocked on the row lock")
+}
+
+// tokenPinnedActions are the two pinned transitions issue #1190 adds callers
+// for: the token cancel and the immediate token approve's claim. Each reports
+// whether it moved the row.
+var tokenPinnedActions = []struct {
+	name   string
+	status string
+	run    func(ctx context.Context, store *PostgresStore, id, rawToken string) (bool, error)
+}{
+	{"cancel", StatusCanceled, func(ctx context.Context, store *PostgresStore, id, rawToken string) (bool, error) {
+		canceled, _, err := pinnedCancel(ctx, store, id, rawToken)
+		return canceled, err
+	}},
+	{"approve claim", "running", func(ctx context.Context, store *PostgresStore, id, rawToken string) (bool, error) {
+		row, err := store.TransitionExecutionStatusTokenPinned(ctx, id, []string{"pending", "notified"}, "running", nil, rawToken)
+		return row != nil, err
+	}},
+}
+
+// The rotation commits while the pinned action is blocked on the row lock:
+// PostgreSQL re-evaluates the action's WHERE against the rotated row, so the
+// old token must lose even though it matched when the action started.
+func TestPostgresStoreDB_TokenPinnedActionBlockedBehindRotationLoses(t *testing.T) {
+	store, newRow := tokenPinFixture(t)
+	ctx := context.Background()
+	for _, action := range tokenPinnedActions {
+		t.Run(action.name, func(t *testing.T) {
+			exec := newRow(t, time.Now().Add(time.Hour))
+			var moved bool
+			var actionErr error
+			done := make(chan struct{})
+			require.NoError(t, store.WithTx(ctx, func(tx pgx.Tx) error {
+				tag, err := tx.Exec(ctx, `UPDATE purchase_executions SET approval_token_hash = $2, status = 'notified'
+					WHERE execution_id = $1 AND status IN ('pending', 'notified')`, exec.ExecutionID, HashApprovalToken(pinNewToken))
+				require.NoError(t, err)
+				require.EqualValues(t, 1, tag.RowsAffected())
+				go func() {
+					defer close(done)
+					moved, actionErr = action.run(ctx, store, exec.ExecutionID, pinOldToken)
+				}()
+				waitForBlockedUpdate(ctx, t, store)
+				return nil
+			}))
+			<-done
+			require.ErrorIs(t, actionErr, ErrApprovalTokenChanged)
+			assert.False(t, moved)
+			requireStillNotified(ctx, t, store, exec.ExecutionID, pinNewToken)
+		})
+	}
+}
+
+// Unsynchronized race: a re-notification rotating the token and an action
+// presenting the old token hit the same row at once, many times. Exactly one
+// may win. Before issue #1190 the cancel and the immediate approve compared
+// the token only against their snapshot, so both could succeed: the row was
+// canceled or claimed with a token that had already been revoked.
+func TestPostgresStoreDB_TokenPinnedActionRacesRotation(t *testing.T) {
+	store, newRow := tokenPinFixture(t)
+	ctx := context.Background()
+	const rounds = 40
+	for _, action := range tokenPinnedActions {
+		t.Run(action.name, func(t *testing.T) {
+			actionWins, rotationWins := 0, 0
+			for i := 0; i < rounds; i++ {
+				exec := newRow(t, time.Now().Add(time.Hour))
+				var wg sync.WaitGroup
+				start := make(chan struct{})
+				var moved, rotated bool
+				var actionErr, rotateErr error
+				// Stagger one side by 0-1.9ms, alternating sides, so both
+				// orderings (and the overlap between them) are exercised.
+				lag := time.Duration(i%20) * 100 * time.Microsecond
+				actionLag, rotateLag := lag, time.Duration(0)
+				if i%2 == 1 {
+					actionLag, rotateLag = 0, lag
+				}
+				wg.Add(2)
+				go func() {
+					defer wg.Done()
+					<-start
+					time.Sleep(actionLag)
+					moved, actionErr = action.run(ctx, store, exec.ExecutionID, pinOldToken)
+				}()
+				go func() {
+					defer wg.Done()
+					<-start
+					time.Sleep(rotateLag)
+					rotated, rotateErr = store.RotatePendingApprovalToken(ctx, exec.ExecutionID, HashApprovalToken(pinNewToken), time.Now().Add(time.Hour))
+				}()
+				close(start)
+				wg.Wait()
+				require.NoError(t, rotateErr)
+				if actionErr != nil {
+					require.ErrorIs(t, actionErr, ErrApprovalTokenChanged, "round %d", i)
+				}
+				require.True(t, moved != rotated, "round %d: exactly one of the action (%v) and the rotation (%v) may win", i, moved, rotated)
+				got, err := store.GetExecutionByID(ctx, exec.ExecutionID)
+				require.NoError(t, err)
+				if moved {
+					actionWins++
+					assert.Equal(t, action.status, got.Status)
+					assert.True(t, ApprovalTokenMatches(got.ApprovalToken, pinOldToken), "a refused rotation must not have written its token")
+				} else {
+					rotationWins++
+					assert.Equal(t, "notified", got.Status, "the rotated row must be left for the new token")
+					assert.True(t, ApprovalTokenMatches(got.ApprovalToken, pinNewToken))
+				}
+			}
+			t.Logf("%s: action won %d, rotation won %d of %d rounds", action.name, actionWins, rotationWins, rounds)
+			assert.Positive(t, actionWins, "the race never let the action go first; the rounds proved nothing about that ordering")
+			assert.Positive(t, rotationWins, "the race never let the rotation go first; the rounds proved nothing about that ordering")
+		})
+	}
+}
diff --git a/internal/mocks/stores.go b/internal/mocks/stores.go
index 38b64862f..30f40d60d 100644
--- a/internal/mocks/stores.go
+++ b/internal/mocks/stores.go
@@ -586,6 +586,17 @@ func (m *MockConfigStore) CancelExecutionAtomic(ctx context.Context, tx pgx.Tx,
 	return args.Bool(0), args.String(1), args.Error(2)
 }
 
+// CancelExecutionAtomicTokenPinned mocks the token-pinned cancel (issue
+// #1190). Like TransitionExecutionStatusTokenPinned it has no default and does
+// not delegate to CancelExecutionAtomic: a caller that must pin the approval
+// token is expected to call this method, so a test has to register it
+// explicitly.
+func (m *MockConfigStore) CancelExecutionAtomicTokenPinned(ctx context.Context, tx pgx.Tx, executionID string, cancelledBy *string, rawToken string) (bool, string, error) { //nolint:gocritic // unnamedResult: return names would conflict with body locals
+	m.record("CancelExecutionAtomicTokenPinned", ctx, tx, executionID, cancelledBy, rawToken)
+	args := m.Called(ctx, tx, executionID, cancelledBy, rawToken)
+	return args.Bool(0), args.String(1), args.Error(2)
+}
+
 // CancelScheduledExecutionAtomic mocks the CancelScheduledExecutionAtomic
 // operation (Gmail-style pre-fire delay revoke, issue #290 wave-2). Default
 // is the happy path (true, "cancelled", nil) so the scheduled-revoke tests
diff --git a/internal/server/test_helpers_test.go b/internal/server/test_helpers_test.go
index f58bee1df..543b287d1 100644
--- a/internal/server/test_helpers_test.go
+++ b/internal/server/test_helpers_test.go
@@ -198,6 +198,10 @@ func (m *mockConfigStoreForHealth) CancelExecutionAtomic(ctx context.Context, tx
 	return false, "", nil
 }
 
+func (m *mockConfigStoreForHealth) CancelExecutionAtomicTokenPinned(ctx context.Context, tx pgx.Tx, executionID string, cancelledBy *string, rawToken string) (bool, string, error) {
+	return false, "", nil
+}
+
 func (m *mockConfigStoreForHealth) CancelScheduledExecutionAtomic(ctx context.Context, tx pgx.Tx, executionID string, cancelledBy *string) (bool, string, error) {
 	return false, "", nil
 }

From b0adbae7ab5c587942c88e83044820d436416141 Mon Sep 17 00:00:00 2001
From: Astrid Gealer <astrid@gealer.email>
Date: Thu, 22 Oct 2026 02:23:32 +0100
Subject: [PATCH 2/3] sec(purchase): pin the approval token in the token
 approve claim and token cancel

The immediate email-link/SQS approve (ApproveExecution) and the email-link/SQS
cancel (CancelExecution) validated the token against a snapshot, then moved
the row with a plain status CAS. A re-notification rotating the token between
the two let the old, revoked token still claim the row to running (and buy)
or cancel it (issue #1190, the #590 class the delayed approve already closed).

ApproveExecution now claims through TransitionExecutionStatusTokenPinned and
CancelExecution cancels through CancelExecutionAtomicTokenPinned, so the token
is re-checked in the same UPDATE as the status change. The session, direct
execute and run-now paths are unchanged (not token-gated). The HTTP layer maps
the lost pin (config.ErrApprovalTokenChanged) to 403 "invalid approval token"
instead of a 500, as the delayed approve already did.

Real-PostgreSQL tests rotate the token between ApproveExecution's/
CancelExecution's snapshot read and their CAS and require the old token to be
refused (both fail on main), plus a cancel-vs-rotation race. Existing mock
tests of the token paths now register the pinned store calls.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 internal/api/handler_purchases.go             |   8 +-
 ...ndler_purchases_cancel_constraints_test.go |   4 +-
 .../handler_purchases_token_errors_test.go    |  38 +++-
 internal/purchase/approvals.go                |  50 +++--
 internal/purchase/approvals_test.go           |  58 +++---
 .../purchase/aws_no_token_redrive_test.go     |  14 +-
 internal/purchase/claim.go                    |  20 +-
 internal/purchase/claimed_row_check_test.go   |  20 +-
 internal/purchase/coverage_extra_test.go      |  14 +-
 internal/purchase/messages_token_ttl_test.go  |   6 +-
 .../token_rotation_pin_integration_test.go    | 182 ++++++++++++++++++
 11 files changed, 339 insertions(+), 75 deletions(-)
 create mode 100644 internal/purchase/token_rotation_pin_integration_test.go

diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go
index af7c61b96..0b0d09fb1 100644
--- a/internal/api/handler_purchases.go
+++ b/internal/api/handler_purchases.go
@@ -909,8 +909,10 @@ func concealExecutionDenial(err error) error {
 
 // tokenActionError maps the email-link approve/cancel failures that are the
 // caller's fault to a 4xx: a wrong token is 403 (as on the RI exchange
-// approve path), an expired token is 410, and an execution
-// whose status no longer allows the action is 409. Anything else stays a server error.
+// approve path), as is a token rotated or expired between the snapshot check
+// and the status CAS (config.ErrApprovalTokenChanged, issue #1190), an expired
+// token is 410, and an execution whose status no longer allows the action is
+// 409. Anything else stays a server error.
 func tokenActionError(err error) error {
 	switch {
 	case errors.Is(err, config.ErrAuditLoss):
@@ -919,7 +921,7 @@ func tokenActionError(err error) error {
 		return claimedRowRefusalError(nil, err)
 	case errors.Is(err, purchase.ErrStaleAzurePricing):
 		return NewClientError(409, err.Error())
-	case errors.Is(err, purchase.ErrInvalidApprovalToken):
+	case errors.Is(err, purchase.ErrInvalidApprovalToken), errors.Is(err, config.ErrApprovalTokenChanged):
 		return NewClientError(403, "invalid approval token")
 	case errors.Is(err, purchase.ErrApprovalTokenExpired):
 		return NewClientError(410, "approval token has expired")
diff --git a/internal/api/handler_purchases_cancel_constraints_test.go b/internal/api/handler_purchases_cancel_constraints_test.go
index 2235f440e..f83f7fcba 100644
--- a/internal/api/handler_purchases_cancel_constraints_test.go
+++ b/internal/api/handler_purchases_cancel_constraints_test.go
@@ -200,6 +200,7 @@ func cancelRouteHandler(exec *config.PurchaseExecution, perms ...auth.Permission
 	store.On("GetExecutionByID", mock.Anything, cancelExecID).Return(exec, nil)
 	store.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{}, nil).Maybe()
 	store.On("CancelExecutionAtomic", mock.Anything, mock.Anything, cancelExecID, mock.Anything).Return(true, "canceled", nil).Maybe()
+	store.On("CancelExecutionAtomicTokenPinned", mock.Anything, mock.Anything, cancelExecID, mock.Anything, mock.Anything).Return(true, "canceled", nil).Maybe()
 	store.On("DeleteSuppressionsByExecutionTx", mock.Anything, mock.Anything, cancelExecID).Return(nil).Maybe()
 	store.On("TransitionExecutionStatus", mock.Anything, cancelExecID, mock.Anything, mock.Anything, mock.Anything).
 		Return(&config.PurchaseExecution{ExecutionID: cancelExecID, Status: "canceled"}, nil).Maybe()
@@ -226,6 +227,7 @@ func TestCancelPurchase_ConstrainedCancelAnyIsDeniedWithAndWithoutToken(t *testi
 			_, err := h.cancelPurchase(context.Background(), sessionCancelReq(), cancelExecID, token)
 			requireRevokeForbidden(t, err, constraintDenied)
 			store.AssertNotCalled(t, "CancelExecutionAtomic", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+			store.AssertNotCalled(t, "CancelExecutionAtomicTokenPinned", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 		})
 	}
 }
@@ -236,7 +238,7 @@ func TestCancelPurchase_ContactApproverWithoutCancelVerbStillUsesToken(t *testin
 	res, err := h.cancelPurchase(context.Background(), sessionCancelReq(), cancelExecID, tokenErrRawToken)
 	require.NoError(t, err)
 	assert.Equal(t, "canceled", res.(map[string]string)["status"])
-	store.AssertCalled(t, "CancelExecutionAtomic", mock.Anything, mock.Anything, cancelExecID, mock.Anything)
+	store.AssertCalled(t, "CancelExecutionAtomicTokenPinned", mock.Anything, mock.Anything, cancelExecID, mock.Anything, tokenErrRawToken)
 }
 
 func TestCancelPurchase_InScopeCancelAnyStillCancels(t *testing.T) {
diff --git a/internal/api/handler_purchases_token_errors_test.go b/internal/api/handler_purchases_token_errors_test.go
index 91d471794..050f02920 100644
--- a/internal/api/handler_purchases_token_errors_test.go
+++ b/internal/api/handler_purchases_token_errors_test.go
@@ -3,6 +3,7 @@ package api
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"testing"
 	"time"
 
@@ -105,7 +106,7 @@ func TestHandleRequest_ApproveViaToken_NonPendingExecutionIs409(t *testing.T) {
 	exec := &config.PurchaseExecution{Status: "completed", ApprovalToken: config.HashApprovalToken(tokenErrRawToken), ApprovalTokenExpiresAt: &future}
 	h := tokenErrHandler(t, exec, nil)
 	store := h.config.(*MockConfigStore)
-	store.On("TransitionExecutionStatus", mock.Anything, tokenErrExecID, mock.Anything, "running", mock.Anything).
+	store.On("TransitionExecutionStatusTokenPinned", mock.Anything, tokenErrExecID, mock.Anything, "running", mock.Anything, tokenErrRawToken).
 		Return(nil, config.ErrExecutionNotInExpectedStatus)
 
 	resp, err := h.HandleRequest(context.Background(), tokenErrRequest("approve", tokenErrRawToken))
@@ -125,10 +126,43 @@ func TestHandleRequest_CancelViaToken_LostCASIs409(t *testing.T) {
 	exec := &config.PurchaseExecution{Status: "pending", ApprovalToken: config.HashApprovalToken(tokenErrRawToken), ApprovalTokenExpiresAt: &future}
 	h := tokenErrHandler(t, exec, nil)
 	store := h.config.(*MockConfigStore)
-	store.On("CancelExecutionAtomic", mock.Anything, mock.Anything, tokenErrExecID, mock.Anything).
+	store.On("CancelExecutionAtomicTokenPinned", mock.Anything, mock.Anything, tokenErrExecID, mock.Anything, tokenErrRawToken).
 		Return(false, "approved", nil)
 
 	resp, err := h.HandleRequest(context.Background(), tokenErrRequest("cancel", tokenErrRawToken))
 	require.NoError(t, err)
 	assert.Equal(t, 409, resp.StatusCode, resp.Body)
 }
+
+// Issue #1190: the email-link approve and cancel pin the approval token inside
+// the status CAS. A token rotated (re-notification) between the snapshot check
+// and the CAS loses the pin with config.ErrApprovalTokenChanged, which is the
+// caller's fault (their link is no longer the current one), so it answers 403
+// "invalid approval token" like a wrong token, not a 500.
+func TestHandleRequest_TokenActions_TokenRotatedBeforeCASIs403(t *testing.T) {
+	future := time.Now().Add(time.Hour)
+	rotated := fmt.Errorf("approve: %w: execution %s", config.ErrApprovalTokenChanged, tokenErrExecID)
+	for _, action := range []string{"approve", "cancel"} {
+		t.Run(action, func(t *testing.T) {
+			exec := &config.PurchaseExecution{Status: "notified", ApprovalToken: config.HashApprovalToken(tokenErrRawToken), ApprovalTokenExpiresAt: &future}
+			h := tokenErrHandler(t, exec, nil)
+			store := h.config.(*MockConfigStore)
+			store.On("TransitionExecutionStatusTokenPinned", mock.Anything, tokenErrExecID, []string{"pending", "notified"}, "running", (*string)(nil), tokenErrRawToken).
+				Return(nil, rotated).Maybe()
+			store.On("CancelExecutionAtomicTokenPinned", mock.Anything, mock.Anything, tokenErrExecID, mock.Anything, tokenErrRawToken).
+				Return(false, "notified", rotated).Maybe()
+
+			resp, err := h.HandleRequest(context.Background(), tokenErrRequest(action, tokenErrRawToken))
+			require.NoError(t, err)
+			assert.Equal(t, 403, resp.StatusCode, resp.Body)
+			assert.Contains(t, resp.Body, "invalid approval token")
+			if action == "approve" {
+				store.AssertCalled(t, "TransitionExecutionStatusTokenPinned", mock.Anything, tokenErrExecID, []string{"pending", "notified"}, "running", (*string)(nil), tokenErrRawToken)
+			} else {
+				store.AssertCalled(t, "CancelExecutionAtomicTokenPinned", mock.Anything, mock.Anything, tokenErrExecID, mock.Anything, tokenErrRawToken)
+			}
+			store.AssertNotCalled(t, "TransitionExecutionStatus", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+			store.AssertNotCalled(t, "CancelExecutionAtomic", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+		})
+	}
+}
diff --git a/internal/purchase/approvals.go b/internal/purchase/approvals.go
index b43544d12..d1de156ae 100644
--- a/internal/purchase/approvals.go
+++ b/internal/purchase/approvals.go
@@ -34,9 +34,10 @@ const directExecuteSkipReason = "direct-execute permission"
 
 // ApproveExecution is the token-authenticated approve entry point used by
 // the legacy email-link flow and the SQS approve worker. After validating
-// the approval token it hands off to ApproveAndExecute, which performs the
-// atomic status transition, runs the AWS purchase synchronously, and mints
-// the fresh revocation token returned here.
+// the approval token it hands off to the ApproveAndExecute funnel
+// (transitionApproveAndExecute), which performs the atomic status transition
+// with the token pinned inside it, runs the AWS purchase synchronously, and
+// mints the fresh revocation token returned here.
 //
 // actor carries the email of the operator who triggered the approval
 // (session-authed click on the HTTP path; verified actor_email on the SQS
@@ -79,7 +80,10 @@ func (m *Manager) ApproveExecution(ctx context.Context, executionID, token, acto
 
 	// Token/SQS path: no authenticated session UUID is available, so the
 	// transition is recorded as system-initiated (transitioned_by = NULL).
-	_, revocationToken, err := m.ApproveAndExecute(ctx, executionID, actor, nil, check)
+	// The token check above read a snapshot; the claim pins the same token
+	// inside its status CAS so a re-notification that rotated it since cannot
+	// win the claim with the old one (issue #1190).
+	_, revocationToken, err := m.transitionApproveAndExecute(ctx, executionID, actor, nil, []string{"pending", "notified"}, "", token, check)
 	if err != nil {
 		logging.Errorf("purchase[%s]: ApproveExecution (token path) failed after %s: %v",
 			executionID, time.Since(t0), err)
@@ -325,7 +329,8 @@ func (m *Manager) checkDifferentApprover(ctx context.Context, executionID string
 // synchronously. Callers MUST have already authorized the actor — this
 // method does no RBAC or token validation; it is shared between:
 //
-//   - ApproveExecution (token/SQS path: token validated by the caller)
+//   - ApproveExecution (token/SQS path: token validated by the caller and
+//     pinned inside the CAS; it calls transitionApproveAndExecute directly)
 //   - approvePurchaseViaSession (session path: RBAC validated by the caller)
 //   - directExecutePurchase (execute_mode="direct": RBAC validated by the caller)
 //
@@ -340,7 +345,7 @@ func (m *Manager) checkDifferentApprover(ctx context.Context, executionID string
 // error; it is nil when the run never started. check runs against the claimed
 // row before any provider call; see ClaimedRowCheck.
 func (m *Manager) ApproveAndExecute(ctx context.Context, executionID, actor string, transitionedBy *string, check ClaimedRowCheck) (*config.PurchaseExecution, string, error) {
-	return m.transitionApproveAndExecute(ctx, executionID, actor, transitionedBy, []string{"pending", "notified"}, "", check)
+	return m.transitionApproveAndExecute(ctx, executionID, actor, transitionedBy, []string{"pending", "notified"}, "", "", check)
 }
 
 // DirectExecute is ApproveAndExecute for a session with the direct-execute
@@ -348,7 +353,7 @@ func (m *Manager) ApproveAndExecute(ctx context.Context, executionID, actor stri
 // executed_* audit fields once the CAS has won, so a refused or lost direct
 // execute leaves no executed_at on a row that never ran.
 func (m *Manager) DirectExecute(ctx context.Context, executionID, actor string, transitionedBy *string, check ClaimedRowCheck) (*config.PurchaseExecution, string, error) {
-	return m.transitionApproveAndExecute(ctx, executionID, actor, transitionedBy, []string{"pending", "notified"}, directExecuteSkipReason, check)
+	return m.transitionApproveAndExecute(ctx, executionID, actor, transitionedBy, []string{"pending", "notified"}, directExecuteSkipReason, "", check)
 }
 
 // RunPlannedPurchaseNow lets an operator force a scheduled purchase (pending
@@ -372,7 +377,7 @@ func (m *Manager) DirectExecute(ctx context.Context, executionID, actor string,
 // on an execution error; it is nil when the run never started. check runs
 // against the claimed row before any provider call; see ClaimedRowCheck.
 func (m *Manager) RunPlannedPurchaseNow(ctx context.Context, executionID, actor string, transitionedBy *string, check ClaimedRowCheck) (*config.PurchaseExecution, string, error) {
-	return m.transitionApproveAndExecute(ctx, executionID, actor, transitionedBy, []string{"pending", "notified", "paused"}, runNowSkipReason, check)
+	return m.transitionApproveAndExecute(ctx, executionID, actor, transitionedBy, []string{"pending", "notified", "paused"}, runNowSkipReason, "", check)
 }
 
 // transitionApproveAndExecute is the shared body behind ApproveAndExecute
@@ -391,12 +396,17 @@ func (m *Manager) RunPlannedPurchaseNow(ctx context.Context, executionID, actor
 // A non-empty skipReason stamps the executed_* audit fields once the CAS wins.
 // The returned row is nil when the CAS was never won.
 //
+// A non-empty approvalToken (ApproveExecution only) is pinned inside the CAS:
+// a token rotated or expired since the caller validated it refuses the claim
+// with config.ErrApprovalTokenChanged and leaves the row untouched (issue
+// #1190). Every other caller passes "".
+//
 // check runs against the row the CAS returned, which is the row
 // executeAndFinalize buys, before any audit stamp or provider call (issue
 // #415). A refusal fails that row before submission (status "failed",
 // no_submission set, a plan row's suppressions released) and returns it with
 // an error satisfying errors.Is(err, ErrClaimedRowRefused).
-func (m *Manager) transitionApproveAndExecute(ctx context.Context, executionID, actor string, transitionedBy *string, fromStatuses []string, skipReason string, check ClaimedRowCheck) (*config.PurchaseExecution, string, error) {
+func (m *Manager) transitionApproveAndExecute(ctx context.Context, executionID, actor string, transitionedBy *string, fromStatuses []string, skipReason, approvalToken string, check ClaimedRowCheck) (*config.PurchaseExecution, string, error) {
 	t0 := time.Now()
 	logging.Infof("purchase[%s]: transitionApproveAndExecute starting (actor=%q, from=%v)", executionID, maskActor(actor), fromStatuses)
 
@@ -419,7 +429,7 @@ func (m *Manager) transitionApproveAndExecute(ctx context.Context, executionID,
 	// executor (SQS, cron, re-drive, scheduler) can run this row, and there is
 	// no window where the row sits "approved" with a stale copy in flight
 	// (issue #829). A lost CAS surfaces ErrExecutionNotInExpectedStatus.
-	updated, err := m.claimAndCheck(ctx, executionID, fromStatuses, transitionedBy, check, t0)
+	updated, err := m.claimAndCheck(ctx, executionID, fromStatuses, transitionedBy, approvalToken, check, t0)
 	if err != nil {
 		return updated, "", err
 	}
@@ -471,12 +481,15 @@ func (m *Manager) transitionApproveAndExecute(ctx context.Context, executionID,
 // verifyAsyncApprovalActor) before reaching here. Same empty-actor
 // rationale as ApproveExecution.
 //
-// Concurrency: CancelExecutionAtomic uses a conditional UPDATE WHERE
-// status IN ('pending','notified') so a concurrent approve that wins
+// Concurrency: CancelExecutionAtomicTokenPinned uses a conditional UPDATE
+// WHERE status IN ('pending','notified') so a concurrent approve that wins
 // the race causes zero rows to be affected and the caller receives a
 // clean error with the current status rather than silently overwriting
-// the approved row. This is the token/email-link cancel analog of
-// the atomic guard TransitionExecutionStatus provides for ApproveAndExecute.
+// the approved row. The same UPDATE also requires the stored token hash to
+// still match token and be unexpired: loadCancelableExecution validated it
+// against a snapshot, and a re-notification that rotated the token since
+// must not let the old one cancel (issue #1190). That refusal returns an
+// error wrapping config.ErrApprovalTokenChanged.
 func (m *Manager) CancelExecution(ctx context.Context, executionID, token, actor string) error {
 	logging.Infof("Canceling execution: %s", executionID)
 
@@ -493,14 +506,15 @@ func (m *Manager) CancelExecution(ctx context.Context, executionID, token, actor
 	}
 
 	// Atomic conditional UPDATE + suppression cleanup in one transaction.
-	// CancelExecutionAtomic flips status only when status IN
-	// ('pending','notified') so a concurrent approve that has already
-	// transitioned the row causes zero rows affected and we surface a 409.
+	// CancelExecutionAtomicTokenPinned flips status only when status IN
+	// ('pending','notified') and the stored token still matches, so a
+	// concurrent approve that has already transitioned the row causes zero
+	// rows affected and we surface a 409, and a rotated token is refused.
 	var canceled bool
 	var currentStatus string
 	if err := m.config.WithTx(ctx, func(tx pgx.Tx) error {
 		var err error
-		canceled, currentStatus, err = m.config.CancelExecutionAtomic(ctx, tx, executionID, cancelledBy)
+		canceled, currentStatus, err = m.config.CancelExecutionAtomicTokenPinned(ctx, tx, executionID, cancelledBy, token)
 		if err != nil {
 			return err
 		}
diff --git a/internal/purchase/approvals_test.go b/internal/purchase/approvals_test.go
index 36cc27ec5..4c60c10c7 100644
--- a/internal/purchase/approvals_test.go
+++ b/internal/purchase/approvals_test.go
@@ -99,7 +99,7 @@ func TestManager_ApproveExecution_Success(t *testing.T) {
 	}
 
 	store.On("GetExecutionByID", ctx, "exec-123").Return(execution, nil)
-	store.On("TransitionExecutionStatus", ctx, "exec-123", approveFromStatuses, "running", (*string)(nil)).Return(updated, nil)
+	store.On("TransitionExecutionStatusTokenPinned", ctx, "exec-123", approveFromStatuses, "running", (*string)(nil), "valid-token").Return(updated, nil)
 	stubExecuteChain(t, store, sender, "plan-456")
 
 	_, err := manager.ApproveExecution(ctx, "exec-123", "valid-token", "", nil)
@@ -128,7 +128,7 @@ func TestManager_ApproveExecution_StampsApprovedBy(t *testing.T) {
 	}
 
 	store.On("GetExecutionByID", ctx, "exec-123").Return(execution, nil)
-	store.On("TransitionExecutionStatus", ctx, "exec-123", approveFromStatuses, "running", (*string)(nil)).Return(updated, nil)
+	store.On("TransitionExecutionStatusTokenPinned", ctx, "exec-123", approveFromStatuses, "running", (*string)(nil), "valid-token").Return(updated, nil)
 	stubExecuteChain(t, store, sender, "plan-456")
 
 	_, err := manager.ApproveExecution(ctx, "exec-123", "valid-token", "operator@example.com", nil)
@@ -163,7 +163,7 @@ func TestManager_ApproveExecution_NotifiedStatus(t *testing.T) {
 	}
 
 	store.On("GetExecutionByID", ctx, "exec-123").Return(execution, nil)
-	store.On("TransitionExecutionStatus", ctx, "exec-123", approveFromStatuses, "running", (*string)(nil)).Return(updated, nil)
+	store.On("TransitionExecutionStatusTokenPinned", ctx, "exec-123", approveFromStatuses, "running", (*string)(nil), "valid-token").Return(updated, nil)
 	stubExecuteChain(t, store, sender, "plan-456")
 
 	_, err := manager.ApproveExecution(ctx, "exec-123", "valid-token", "", nil)
@@ -191,7 +191,7 @@ func TestManager_ApproveExecution_InvalidToken(t *testing.T) {
 	// Critically: TransitionExecutionStatus and SavePurchaseExecution must
 	// NOT have been called. A token-validation bypass is exactly what the
 	// constant-time comparison guards against.
-	store.AssertNotCalled(t, "TransitionExecutionStatus", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	store.AssertNotCalled(t, "TransitionExecutionStatusTokenPinned", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 	store.AssertNotCalled(t, "SavePurchaseExecution", mock.Anything, mock.Anything)
 	store.AssertExpectations(t)
 }
@@ -210,7 +210,7 @@ func TestManager_ApproveExecution_EmptyToken(t *testing.T) {
 	_, err := manager.ApproveExecution(ctx, "exec-123", "", "", nil)
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "invalid approval token")
-	store.AssertNotCalled(t, "TransitionExecutionStatus", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	store.AssertNotCalled(t, "TransitionExecutionStatusTokenPinned", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 }
 
 func TestManager_ApproveExecution_NotFound(t *testing.T) {
@@ -252,7 +252,7 @@ func TestManager_ApproveExecution_TransitionFails(t *testing.T) {
 		ApprovalToken: config.HashApprovalToken("valid-token"),
 	}
 	store.On("GetExecutionByID", ctx, "exec-123").Return(execution, nil)
-	store.On("TransitionExecutionStatus", ctx, "exec-123", approveFromStatuses, "running", (*string)(nil)).
+	store.On("TransitionExecutionStatusTokenPinned", ctx, "exec-123", approveFromStatuses, "running", (*string)(nil), "valid-token").
 		Return(nil, errors.New(`execution exec-123 cannot transition from "canceled" to "approved"`))
 
 	_, err := manager.ApproveExecution(ctx, "exec-123", "valid-token", "", nil)
@@ -703,8 +703,8 @@ func TestManager_ApproveExecution_FourEyesOn_SQSActorEqualsCreator_Denied(t *tes
 			_, err := manager.ApproveExecution(ctx, "exec-sqs-self", "valid-token", tc.actorEmail, nil)
 			require.ErrorIs(t, err, ErrFourEyesDenied)
 			assert.Contains(t, err.Error(), "4-eyes mode requires a different approver")
-			store.AssertNotCalled(t, "TransitionExecutionStatus",
-				mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+			store.AssertNotCalled(t, "TransitionExecutionStatusTokenPinned",
+				mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 			store.AssertExpectations(t)
 		})
 	}
@@ -737,7 +737,7 @@ func TestManager_ApproveExecution_FourEyesOn_DifferentActor_Allowed(t *testing.T
 	store.On("GetExecutionByID", ctx, "exec-sqs-diff").Return(execution, nil)
 	store.On("GetGlobalConfig", mock.Anything).Return(fourEyesCfgOnForManager(), nil)
 	store.On("GetUserEmailByID", ctx, creatorID).Return("creator@example.com", nil)
-	store.On("TransitionExecutionStatus", ctx, "exec-sqs-diff", approveFromStatuses, "running", (*string)(nil)).Return(updated, nil)
+	store.On("TransitionExecutionStatusTokenPinned", ctx, "exec-sqs-diff", approveFromStatuses, "running", (*string)(nil), "valid-token").Return(updated, nil)
 	stubExecuteChain(t, store, sender, "plan-fourEyes")
 
 	_, err := manager.ApproveExecution(ctx, "exec-sqs-diff", "valid-token", "approver@example.com", nil)
@@ -784,8 +784,8 @@ func TestManager_ApproveExecution_FourEyesOn_UnresolvableCreator_Denied(t *testi
 		_, err := manager.ApproveExecution(ctx, "exec-sqs-unresolvable", "valid-token", "approver@example.com", nil)
 		require.Error(t, err)
 		assert.Contains(t, err.Error(), "creator's account could not be resolved")
-		store.AssertNotCalled(t, "TransitionExecutionStatus",
-			mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+		store.AssertNotCalled(t, "TransitionExecutionStatusTokenPinned",
+			mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 		store.AssertExpectations(t)
 	})
 
@@ -801,8 +801,8 @@ func TestManager_ApproveExecution_FourEyesOn_UnresolvableCreator_Denied(t *testi
 		_, err := manager.ApproveExecution(ctx, "exec-sqs-unresolvable", "valid-token", "approver@example.com", nil)
 		require.Error(t, err)
 		assert.Contains(t, err.Error(), "failed to resolve creator identity")
-		store.AssertNotCalled(t, "TransitionExecutionStatus",
-			mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+		store.AssertNotCalled(t, "TransitionExecutionStatusTokenPinned",
+			mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 		store.AssertExpectations(t)
 	})
 }
@@ -821,7 +821,7 @@ func TestManager_CancelExecution(t *testing.T) {
 
 	mockStore.On("GetExecutionByID", ctx, "exec-123").Return(execution, nil)
 	// WithTx passes nil as the tx sentinel in tests; empty actor -> nil cancelledBy.
-	mockStore.On("CancelExecutionAtomic", ctx, mock.Anything, "exec-123", (*string)(nil)).
+	mockStore.On("CancelExecutionAtomicTokenPinned", ctx, mock.Anything, "exec-123", (*string)(nil), "valid-token").
 		Return(true, "canceled", nil)
 	mockStore.On("DeleteSuppressionsByExecutionTx", ctx, mock.Anything, "exec-123").
 		Return(nil)
@@ -937,8 +937,8 @@ func TestManager_CancelExecution_RejectsNonCancelableStatus(t *testing.T) {
 				assert.Contains(t, err.Error(), "use the revoke endpoint before it fires")
 			}
 			// Status guard fires before the atomic UPDATE — a rejected
-			// cancel must never reach CancelExecutionAtomic.
-			mockStore.AssertNotCalled(t, "CancelExecutionAtomic", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+			// cancel must never reach CancelExecutionAtomicTokenPinned.
+			mockStore.AssertNotCalled(t, "CancelExecutionAtomicTokenPinned", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 			mockStore.AssertExpectations(t)
 		})
 	}
@@ -964,8 +964,8 @@ func TestManager_CancelExecution_AllowsCancelableStatus(t *testing.T) {
 				ApprovalToken: config.HashApprovalToken("valid-token"),
 			}
 			mockStore.On("GetExecutionByID", ctx, "exec-123").Return(execution, nil)
-			// CancelExecutionAtomic is called inside WithTx (nil tx sentinel in tests).
-			mockStore.On("CancelExecutionAtomic", ctx, mock.Anything, "exec-123", (*string)(nil)).
+			// CancelExecutionAtomicTokenPinned is called inside WithTx (nil tx sentinel in tests).
+			mockStore.On("CancelExecutionAtomicTokenPinned", ctx, mock.Anything, "exec-123", (*string)(nil), "valid-token").
 				Return(true, "canceled", nil)
 			// Suppression cleanup must follow a successful atomic cancel.
 			mockStore.On("DeleteSuppressionsByExecutionTx", ctx, mock.Anything, "exec-123").
@@ -981,7 +981,7 @@ func TestManager_CancelExecution_AllowsCancelableStatus(t *testing.T) {
 
 			err := manager.CancelExecution(ctx, "exec-123", "valid-token", "")
 			require.NoError(t, err)
-			mockStore.AssertCalled(t, "CancelExecutionAtomic", ctx, mock.Anything, "exec-123", (*string)(nil))
+			mockStore.AssertCalled(t, "CancelExecutionAtomicTokenPinned", ctx, mock.Anything, "exec-123", (*string)(nil), "valid-token")
 			mockStore.AssertCalled(t, "DeleteSuppressionsByExecutionTx", ctx, mock.Anything, "exec-123")
 			mockStore.AssertExpectations(t)
 		})
@@ -1053,7 +1053,7 @@ func TestManager_ApproveExecution_ExpiredToken(t *testing.T) {
 	assert.Contains(t, err.Error(), "expired")
 	// Token validation passes but the expiry check fires — TransitionExecutionStatus
 	// must never be called on an expired token.
-	store.AssertNotCalled(t, "TransitionExecutionStatus", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	store.AssertNotCalled(t, "TransitionExecutionStatusTokenPinned", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 	store.AssertExpectations(t)
 }
 
@@ -1080,7 +1080,7 @@ func TestManager_ApproveExecution_ValidTokenWithinTTL(t *testing.T) {
 		Recommendations: approvalTestRecs(),
 	}
 	store.On("GetExecutionByID", ctx, "exec-live").Return(execution, nil)
-	store.On("TransitionExecutionStatus", ctx, "exec-live", approveFromStatuses, "running", (*string)(nil)).Return(updated, nil)
+	store.On("TransitionExecutionStatusTokenPinned", ctx, "exec-live", approveFromStatuses, "running", (*string)(nil), "valid-token").Return(updated, nil)
 	stubExecuteChain(t, store, sender, "plan-live")
 
 	_, err := manager.ApproveExecution(ctx, "exec-live", "valid-token", "", nil)
@@ -1111,7 +1111,7 @@ func TestManager_ApproveExecution_NilExpiresAt_LegacyRow(t *testing.T) {
 		Recommendations: approvalTestRecs(),
 	}
 	store.On("GetExecutionByID", ctx, "exec-legacy").Return(execution, nil)
-	store.On("TransitionExecutionStatus", ctx, "exec-legacy", approveFromStatuses, "running", (*string)(nil)).Return(updated, nil)
+	store.On("TransitionExecutionStatusTokenPinned", ctx, "exec-legacy", approveFromStatuses, "running", (*string)(nil), "valid-token").Return(updated, nil)
 	stubExecuteChain(t, store, sender, "plan-legacy")
 
 	_, err := manager.ApproveExecution(ctx, "exec-legacy", "valid-token", "", nil)
@@ -1145,7 +1145,7 @@ func TestManager_ApproveExecution_NonAWSOrphanReturnsError(t *testing.T) {
 	assert.Contains(t, err.Error(), "no longer exists")
 	assert.Contains(t, err.Error(), "azure")
 	// Execute chain must not run after the orphan guard fires.
-	store.AssertNotCalled(t, "TransitionExecutionStatus", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	store.AssertNotCalled(t, "TransitionExecutionStatusTokenPinned", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 	store.AssertExpectations(t)
 }
 
@@ -1173,7 +1173,7 @@ func TestManager_ApproveExecution_AWSOrphanFallsThrough(t *testing.T) {
 		Recommendations: approvalTestRecs(),
 	}
 	store.On("GetExecutionByID", ctx, "exec-aws-ambient").Return(execution, nil)
-	store.On("TransitionExecutionStatus", ctx, "exec-aws-ambient", approveFromStatuses, "running", (*string)(nil)).Return(updated, nil)
+	store.On("TransitionExecutionStatusTokenPinned", ctx, "exec-aws-ambient", approveFromStatuses, "running", (*string)(nil), "valid-token").Return(updated, nil)
 	stubExecuteChain(t, store, sender, "plan-aws")
 
 	_, err := manager.ApproveExecution(ctx, "exec-aws-ambient", "valid-token", "", nil)
@@ -1207,7 +1207,7 @@ func TestOrphanExecutionError_RecLevelAccountIDPreventsOrphan(t *testing.T) {
 // TestManager_CancelExecution_RaceWithApprove is the regression guard for
 // issue #671: when a concurrent approve wins the race and transitions the
 // execution to 'approved' before the cancel's conditional UPDATE runs,
-// CancelExecutionAtomic returns (false, "approved", nil). CancelExecution
+// CancelExecutionAtomicTokenPinned returns (false, "approved", nil). CancelExecution
 // must surface a clean error containing the racing status rather than
 // silently overwriting the approved row.
 func TestManager_CancelExecution_RaceWithApprove(t *testing.T) {
@@ -1222,7 +1222,7 @@ func TestManager_CancelExecution_RaceWithApprove(t *testing.T) {
 	store.On("GetExecutionByID", ctx, "exec-raced").Return(execution, nil)
 	// Simulate: concurrent approve won between our IsCancelable check and
 	// the atomic UPDATE. The DB row is now 'approved' so zero rows affected.
-	store.On("CancelExecutionAtomic", ctx, mock.Anything, "exec-raced", (*string)(nil)).
+	store.On("CancelExecutionAtomicTokenPinned", ctx, mock.Anything, "exec-raced", (*string)(nil), "valid-token").
 		Return(false, "approved", nil)
 
 	err := manager.CancelExecution(ctx, "exec-raced", "valid-token", "")
@@ -1231,7 +1231,7 @@ func TestManager_CancelExecution_RaceWithApprove(t *testing.T) {
 	assert.Contains(t, err.Error(), "concurrent", "error must mention the concurrent operation")
 	// The approve is already in flight — the DB row must not have been
 	// overwritten by the cancel. AssertExpectations verifies that
-	// CancelExecutionAtomic was called (confirming the guard reached the DB)
+	// CancelExecutionAtomicTokenPinned was called (confirming the guard reached the DB)
 	// and that no further writes landed.
 	store.AssertExpectations(t)
 	// When the CAS misses, suppression cleanup must never fire.
@@ -1256,7 +1256,7 @@ func TestManager_CancelExecution_ExpiredToken(t *testing.T) {
 	err := manager.CancelExecution(ctx, "exec-cancel-expired", "valid-token", "")
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "expired")
-	store.AssertNotCalled(t, "CancelExecutionAtomic", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	store.AssertNotCalled(t, "CancelExecutionAtomicTokenPinned", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 	store.AssertExpectations(t)
 }
 
@@ -1293,7 +1293,7 @@ func TestApproveExecution_MintsRevocationToken(t *testing.T) {
 	// returned and executeAndFinalize continued operating on -- in place; it
 	// does no fetch of its own (see rotateApprovalToken's doc comment for why).
 	store.On("GetExecutionByID", ctx, "exec-rotate").Return(execution, nil)
-	store.On("TransitionExecutionStatus", ctx, "exec-rotate", approveFromStatuses, "running", (*string)(nil)).Return(updated, nil)
+	store.On("TransitionExecutionStatusTokenPinned", ctx, "exec-rotate", approveFromStatuses, "running", (*string)(nil), "pre-rotate-token").Return(updated, nil)
 	stubExecuteChain(t, store, sender, "plan-rotate")
 
 	_, err := manager.ApproveExecution(ctx, "exec-rotate", "pre-rotate-token", "", nil)
diff --git a/internal/purchase/aws_no_token_redrive_test.go b/internal/purchase/aws_no_token_redrive_test.go
index 4acddfa7c..3193d7fbe 100644
--- a/internal/purchase/aws_no_token_redrive_test.go
+++ b/internal/purchase/aws_no_token_redrive_test.go
@@ -178,13 +178,14 @@ func TestApproveAndExecuteRefusesAWSNoTokenRetrySuccessor(t *testing.T) {
 	paths := []struct {
 		name           string
 		transitionedBy *string
+		pinnedToken    string // the token path pins its token inside the claim (issue #1190)
 		approve        func(m *Manager, id string) error
 	}{
-		{"token", nil, func(m *Manager, id string) error {
+		{"token", nil, rawToken, func(m *Manager, id string) error {
 			_, err := m.ApproveExecution(context.Background(), id, rawToken, "approver@example.com", nil)
 			return err
 		}},
-		{"session", &sessionUser, func(m *Manager, id string) error {
+		{"session", &sessionUser, "", func(m *Manager, id string) error {
 			_, _, err := m.ApproveAndExecute(context.Background(), id, "approver@example.com", &sessionUser, nil)
 			return err
 		}},
@@ -207,8 +208,13 @@ func TestApproveAndExecuteRefusesAWSNoTokenRetrySuccessor(t *testing.T) {
 				store.On("GetExecutionByID", mock.Anything, exec.ExecutionID).Return(exec, nil).Maybe()
 				claimed := *exec
 				claimed.Status = "running"
-				store.On("TransitionExecutionStatus", mock.Anything, exec.ExecutionID,
-					[]string{"pending", "notified"}, "running", path.transitionedBy).Return(&claimed, nil).Once()
+				if path.pinnedToken != "" {
+					store.On("TransitionExecutionStatusTokenPinned", mock.Anything, exec.ExecutionID,
+						[]string{"pending", "notified"}, "running", path.transitionedBy, path.pinnedToken).Return(&claimed, nil).Once()
+				} else {
+					store.On("TransitionExecutionStatus", mock.Anything, exec.ExecutionID,
+						[]string{"pending", "notified"}, "running", path.transitionedBy).Return(&claimed, nil).Once()
+				}
 
 				err := path.approve(mgr, exec.ExecutionID)
 
diff --git a/internal/purchase/claim.go b/internal/purchase/claim.go
index 70d6a2216..66d0ebba4 100644
--- a/internal/purchase/claim.go
+++ b/internal/purchase/claim.go
@@ -40,12 +40,28 @@ func (m *Manager) claimRunning(ctx context.Context, executionID string, from []s
 	return m.config.TransitionExecutionStatus(ctx, executionID, from, "running", actor)
 }
 
+// claimRunningForApprove is claimRunning for transitionApproveAndExecute. A
+// non-empty approvalToken (the email-link and SQS token approve) is pinned in
+// the same UPDATE as the claim: the row's stored token hash must still match
+// it and be unexpired, so a re-notification that rotated the token after
+// ApproveExecution validated its snapshot refuses the claim with
+// config.ErrApprovalTokenChanged instead of letting the old, revoked token buy
+// (issue #1190, the immediate-approve side of #590). Session, direct-execute
+// and run-now callers pass "" and are not token-gated.
+func (m *Manager) claimRunningForApprove(ctx context.Context, executionID string, from []string, actor *string, approvalToken string) (*config.PurchaseExecution, error) {
+	if approvalToken == "" {
+		return m.claimRunning(ctx, executionID, from, actor)
+	}
+	return m.config.TransitionExecutionStatusTokenPinned(ctx, executionID, from, "running", actor, approvalToken)
+}
+
 // claimAndCheck claims the row for transitionApproveAndExecute and then runs
 // check against the claimed row. A lost claim returns (nil, err). A refused
 // check fails the claimed row before submission and returns (row, err), with
 // err satisfying errors.Is(err, ErrClaimedRowRefused); nothing is bought.
-func (m *Manager) claimAndCheck(ctx context.Context, executionID string, from []string, actor *string, check ClaimedRowCheck, t0 time.Time) (*config.PurchaseExecution, error) {
-	updated, err := m.claimRunning(ctx, executionID, from, actor)
+// approvalToken is pinned inside the claim; see claimRunningForApprove.
+func (m *Manager) claimAndCheck(ctx context.Context, executionID string, from []string, actor *string, approvalToken string, check ClaimedRowCheck, t0 time.Time) (*config.PurchaseExecution, error) {
+	updated, err := m.claimRunningForApprove(ctx, executionID, from, actor, approvalToken)
 	if err != nil {
 		logging.Errorf("purchase[%s]: transitionApproveAndExecute status transition failed after %s: %v",
 			executionID, time.Since(t0), err)
diff --git a/internal/purchase/claimed_row_check_test.go b/internal/purchase/claimed_row_check_test.go
index fd1a723fd..22c6c4ce6 100644
--- a/internal/purchase/claimed_row_check_test.go
+++ b/internal/purchase/claimed_row_check_test.go
@@ -59,6 +59,9 @@ type claimEntryPoint struct {
 	name string
 	from []string
 	run  func(m *Manager, id string, check ClaimedRowCheck) (*config.PurchaseExecution, error)
+	// pinnedToken is the approval token the entry point pins inside its claim
+	// (the token approve, issue #1190); "" for the unpinned entry points.
+	pinnedToken string
 }
 
 func claimEntryPoints() []claimEntryPoint {
@@ -68,19 +71,19 @@ func claimEntryPoints() []claimEntryPoint {
 		{"run-now", runNowFrom, func(m *Manager, id string, c ClaimedRowCheck) (*config.PurchaseExecution, error) {
 			row, _, err := m.RunPlannedPurchaseNow(ctx, id, actor, nil, c)
 			return row, err
-		}},
+		}, ""},
 		{"session approve", []string{"pending", "notified"}, func(m *Manager, id string, c ClaimedRowCheck) (*config.PurchaseExecution, error) {
 			row, _, err := m.ApproveAndExecute(ctx, id, actor, nil, c)
 			return row, err
-		}},
+		}, ""},
 		{"direct execute", []string{"pending", "notified"}, func(m *Manager, id string, c ClaimedRowCheck) (*config.PurchaseExecution, error) {
 			row, _, err := m.DirectExecute(ctx, id, actor, nil, c)
 			return row, err
-		}},
+		}, ""},
 		{"token approve", []string{"pending", "notified"}, func(m *Manager, id string, c ClaimedRowCheck) (*config.PurchaseExecution, error) {
 			_, err := m.ApproveExecution(ctx, id, claimCheckToken, actor, c)
 			return nil, err
-		}},
+		}, claimCheckToken},
 	}
 }
 
@@ -104,8 +107,13 @@ func claimCheckHarness(t *testing.T, ep claimEntryPoint, claimedPrice float64) (
 	claimed.Recommendations = append([]config.RecommendationRecord(nil), snapshot.Recommendations...)
 	claimed.Recommendations[0].UpfrontCost = claimedPrice
 	claimed.Status = "running"
-	store.On("TransitionExecutionStatus", mock.Anything, snapshot.ExecutionID, ep.from, "running", (*string)(nil)).
-		Return(&claimed, nil).Once()
+	if ep.pinnedToken != "" {
+		store.On("TransitionExecutionStatusTokenPinned", mock.Anything, snapshot.ExecutionID, ep.from, "running", (*string)(nil), ep.pinnedToken).
+			Return(&claimed, nil).Once()
+	} else {
+		store.On("TransitionExecutionStatus", mock.Anything, snapshot.ExecutionID, ep.from, "running", (*string)(nil)).
+			Return(&claimed, nil).Once()
+	}
 	return mgr, store, rec, snapshot, saved
 }
 
diff --git a/internal/purchase/coverage_extra_test.go b/internal/purchase/coverage_extra_test.go
index 3737064d6..b477621ae 100644
--- a/internal/purchase/coverage_extra_test.go
+++ b/internal/purchase/coverage_extra_test.go
@@ -405,7 +405,7 @@ func TestProcessMessage_ApproveHappyPath(t *testing.T) {
 	mockStore.On("GetExecutionByID", ctx, "exec-appv").Return(exec, nil).Times(2)
 	mockStore.On("GetCloudAccount", ctx, accountID).Return(account, nil)
 	// Atomic approve transition (issue #372 fix).
-	mockStore.On("TransitionExecutionStatus", ctx, "exec-appv", []string{"pending", "notified"}, "running", (*string)(nil)).Return(approved, nil)
+	mockStore.On("TransitionExecutionStatusTokenPinned", ctx, "exec-appv", []string{"pending", "notified"}, "running", (*string)(nil), "correct-token").Return(approved, nil)
 	// Synchronous execute chain: GetPurchasePlan is called with the
 	// approved execution's PlanID — pinning the non-empty id here means a
 	// regression that drops it (e.g. passes "" or exec.ExecutionID by
@@ -490,8 +490,8 @@ func TestProcessMessage_ApproveFourEyesOn_SelfApproveDenied(t *testing.T) {
 			err := manager.ProcessMessage(ctx, msg)
 			require.ErrorIs(t, err, ErrFourEyesDenied)
 			assert.Contains(t, err.Error(), "4-eyes mode requires a different approver")
-			mockStore.AssertNotCalled(t, "TransitionExecutionStatus",
-				mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+			mockStore.AssertNotCalled(t, "TransitionExecutionStatusTokenPinned",
+				mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 			mockStore.AssertExpectations(t)
 			mockEmail.AssertExpectations(t)
 		})
@@ -582,7 +582,7 @@ func TestProcessMessage_ApproveFourEyesOn_DifferentApproverSucceeds(t *testing.T
 	mockStore.On("GetCloudAccount", ctx, accountID).Return(account, nil)
 	mockStore.On("GetGlobalConfig", mock.Anything).Return(fourEyesCfgOnForManager(), nil)
 	mockStore.On("GetUserEmailByID", ctx, creatorID).Return("owner@example.com", nil)
-	mockStore.On("TransitionExecutionStatus", ctx, "exec-appv-diff", []string{"pending", "notified"}, "running", (*string)(nil)).Return(approved, nil)
+	mockStore.On("TransitionExecutionStatusTokenPinned", ctx, "exec-appv-diff", []string{"pending", "notified"}, "running", (*string)(nil), "correct-token").Return(approved, nil)
 	plan := &config.PurchasePlan{ID: planID, Name: "test-plan"}
 	mockStore.On("GetPurchasePlan", mock.Anything, planID).Return(plan, nil)
 	mockEmail.On("SendPurchaseConfirmation", mock.Anything, mock.Anything).Return(nil)
@@ -621,10 +621,10 @@ func TestProcessMessage_CancelHappyPath(t *testing.T) {
 	// load the execution; mock returns it three times.
 	mockStore.On("GetExecutionByID", ctx, "exec-cancel").Return(exec, nil).Times(3)
 	mockStore.On("GetCloudAccount", ctx, accountID).Return(account, nil)
-	// CancelExecutionAtomic is called inside WithTx (nil tx sentinel in
-	// tests); actor_email is non-empty so cancelledBy is non-nil.
+	// CancelExecutionAtomicTokenPinned is called inside WithTx (nil tx
+	// sentinel in tests); actor_email is non-empty so cancelledBy is non-nil.
 	actor := "owner@example.com"
-	mockStore.On("CancelExecutionAtomic", ctx, mock.Anything, "exec-cancel", &actor).
+	mockStore.On("CancelExecutionAtomicTokenPinned", ctx, mock.Anything, "exec-cancel", &actor, "correct-token").
 		Return(true, "canceled", nil)
 	// Suppression cleanup must follow a successful atomic cancel.
 	mockStore.On("DeleteSuppressionsByExecutionTx", ctx, mock.Anything, "exec-cancel").
diff --git a/internal/purchase/messages_token_ttl_test.go b/internal/purchase/messages_token_ttl_test.go
index c8ea984b1..3b0750619 100644
--- a/internal/purchase/messages_token_ttl_test.go
+++ b/internal/purchase/messages_token_ttl_test.go
@@ -109,10 +109,10 @@ func TestProcessMessage_ExpiredTokenRejectedBeforeApproverLookup(t *testing.T) {
 			require.ErrorIs(t, err, ErrApprovalTokenExpired)
 			mockStore.AssertNumberOfCalls(t, "GetExecutionByID", 1)
 			mockStore.AssertNotCalled(t, "GetCloudAccount", mock.Anything, mock.Anything)
-			mockStore.AssertNotCalled(t, "TransitionExecutionStatus",
+			mockStore.AssertNotCalled(t, "TransitionExecutionStatusTokenPinned",
+				mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+			mockStore.AssertNotCalled(t, "CancelExecutionAtomicTokenPinned",
 				mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
-			mockStore.AssertNotCalled(t, "CancelExecutionAtomic",
-				mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 			mockStore.AssertNotCalled(t, "SavePurchaseExecution", mock.Anything, mock.Anything)
 			mockStore.AssertExpectations(t)
 		})
diff --git a/internal/purchase/token_rotation_pin_integration_test.go b/internal/purchase/token_rotation_pin_integration_test.go
new file mode 100644
index 000000000..8b148e034
--- /dev/null
+++ b/internal/purchase/token_rotation_pin_integration_test.go
@@ -0,0 +1,182 @@
+//go:build integration
+
+package purchase
+
+import (
+	"context"
+	"errors"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+// Real PostgreSQL, mock email sender and mock provider factory (labeled
+// fixtures; nothing here reaches a cloud API). Issue #1190: the immediate
+// email-link approve and the email-link cancel validated the approval token
+// against a snapshot and then moved the row with a plain status CAS, so a
+// re-notification rotating the token between the two let the old, revoked
+// token still approve (and buy) or cancel.
+
+const rotOldToken, rotNewToken = "rot-old-raw-token", "rot-new-raw-token"
+
+// rotatingStore rotates the row's approval token, as a re-notification does,
+// right after the first read of the row returns: the caller validates the
+// old token against that snapshot and only then reaches its status CAS.
+type rotatingStore struct {
+	*config.PostgresStore
+	t    *testing.T
+	once sync.Once
+}
+
+func (r *rotatingStore) GetExecutionByID(ctx context.Context, id string) (*config.PurchaseExecution, error) {
+	snapshot, err := r.PostgresStore.GetExecutionByID(ctx, id)
+	r.once.Do(func() {
+		rotated, rotErr := r.RotatePendingApprovalToken(ctx, id, config.HashApprovalToken(rotNewToken), time.Now().Add(time.Hour))
+		require.NoError(r.t, rotErr)
+		require.True(r.t, rotated, "the fixture row must be rotatable")
+	})
+	return snapshot, err
+}
+
+func tokenRotationRow(ctx context.Context, t *testing.T, store *config.PostgresStore, planID string) *config.PurchaseExecution {
+	t.Helper()
+	expires := time.Now().Add(time.Hour)
+	row := &config.PurchaseExecution{
+		PlanID: planID, ExecutionID: uuid.New().String(), Status: "notified", StepNumber: 1,
+		ScheduledDate: time.Now().Add(time.Hour), Recommendations: []config.RecommendationRecord{upfrontRec(10)},
+		ApprovalToken: config.HashApprovalToken(rotOldToken), ApprovalTokenExpiresAt: &expires,
+	}
+	require.NoError(t, store.SavePurchaseExecution(ctx, row))
+	return row
+}
+
+// stubUnreachableProvider makes the factory's provider call fail; the tests
+// assert it is never called on a refused row.
+func stubUnreachableProvider(factory *MockProviderFactory) {
+	factory.On("CreateAndValidateProvider", mock.Anything, mock.Anything, mock.Anything).
+		Return(nil, errors.New("fixture: no cloud provider in this test")).Maybe()
+}
+
+// quietSender accepts the outcome emails a run that reached execution sends.
+func quietSender() *MockEmailSender {
+	sender := new(MockEmailSender)
+	sender.On("SendPurchaseConfirmation", mock.Anything, mock.Anything).Return(nil).Maybe()
+	sender.On("SendPurchaseFailedNotification", mock.Anything, mock.Anything).Return(nil).Maybe()
+	return sender
+}
+
+func TestApproveExecution_TokenRotatedBeforeClaimIsRefused(t *testing.T) {
+	ctx := context.Background()
+	store, _ := rampStepStore(ctx, t)
+	plan := atomicStepPlan(ctx, t, store)
+
+	t.Run("old token after a rotation", func(t *testing.T) {
+		row := tokenRotationRow(ctx, t, store, plan.ID)
+		factory := new(MockProviderFactory)
+		stubUnreachableProvider(factory)
+		manager := &Manager{config: &rotatingStore{PostgresStore: store, t: t}, email: quietSender(), providerFactory: factory}
+
+		revocationToken, err := manager.ApproveExecution(ctx, row.ExecutionID, rotOldToken, "approver@example.invalid", nil)
+
+		require.ErrorIs(t, err, config.ErrApprovalTokenChanged, "the revoked token must lose the claim")
+		assert.Empty(t, revocationToken)
+		after := statusInDB(ctx, t, store, row.ExecutionID)
+		assert.Equal(t, "notified", after.Status, "the row is left for the current token")
+		assert.True(t, config.ApprovalTokenMatches(after.ApprovalToken, rotNewToken), "the rotated token is kept")
+		assert.Nil(t, after.ApprovedBy)
+		factory.AssertNotCalled(t, "CreateAndValidateProvider", mock.Anything, mock.Anything, mock.Anything)
+	})
+
+	t.Run("positive control: the same token with no rotation wins the claim", func(t *testing.T) {
+		row := tokenRotationRow(ctx, t, store, plan.ID)
+		factory := new(MockProviderFactory)
+		stubUnreachableProvider(factory)
+		manager := &Manager{config: store, email: quietSender(), providerFactory: factory}
+
+		_, err := manager.ApproveExecution(ctx, row.ExecutionID, rotOldToken, "approver@example.invalid", nil)
+
+		require.NotErrorIs(t, err, config.ErrApprovalTokenChanged)
+		assert.NotEqual(t, "notified", statusInDB(ctx, t, store, row.ExecutionID).Status, "the claim was won and the run started")
+	})
+}
+
+func TestCancelExecution_TokenRotatedBeforeCancelIsRefused(t *testing.T) {
+	ctx := context.Background()
+	store, _ := rampStepStore(ctx, t)
+	plan := atomicStepPlan(ctx, t, store)
+	row := tokenRotationRow(ctx, t, store, plan.ID)
+	manager := &Manager{config: &rotatingStore{PostgresStore: store, t: t}}
+
+	err := manager.CancelExecution(ctx, row.ExecutionID, rotOldToken, "a@example.invalid")
+
+	require.ErrorIs(t, err, config.ErrApprovalTokenChanged, "the revoked token must not cancel")
+	after := statusInDB(ctx, t, store, row.ExecutionID)
+	assert.Equal(t, "notified", after.Status)
+	assert.True(t, config.ApprovalTokenMatches(after.ApprovalToken, rotNewToken))
+
+	// The current token still cancels.
+	require.NoError(t, manager.CancelExecution(ctx, row.ExecutionID, rotNewToken, "a@example.invalid"))
+	assert.Equal(t, config.StatusCanceled, statusInDB(ctx, t, store, row.ExecutionID).Status)
+}
+
+// The full email-link cancel (snapshot check, then pinned cancel) racing a
+// re-notification, many times: exactly one of them may take effect.
+func TestCancelExecution_RacesRotation(t *testing.T) {
+	ctx := context.Background()
+	store, _ := rampStepStore(ctx, t)
+	plan := atomicStepPlan(ctx, t, store)
+	manager := &Manager{config: store}
+	const rounds = 40
+	cancelWins, rotationWins := 0, 0
+	for i := 0; i < rounds; i++ {
+		row := tokenRotationRow(ctx, t, store, plan.ID)
+		lag := time.Duration(i%20) * 100 * time.Microsecond
+		cancelLag, rotateLag := lag, time.Duration(0)
+		if i%2 == 1 {
+			cancelLag, rotateLag = 0, lag
+		}
+		var wg sync.WaitGroup
+		start := make(chan struct{})
+		var cancelErr, rotateErr error
+		var rotated bool
+		wg.Add(2)
+		go func() {
+			defer wg.Done()
+			<-start
+			time.Sleep(cancelLag)
+			cancelErr = manager.CancelExecution(ctx, row.ExecutionID, rotOldToken, "a@example.invalid")
+		}()
+		go func() {
+			defer wg.Done()
+			<-start
+			time.Sleep(rotateLag)
+			rotated, rotateErr = store.RotatePendingApprovalToken(ctx, row.ExecutionID, config.HashApprovalToken(rotNewToken), time.Now().Add(time.Hour))
+		}()
+		close(start)
+		wg.Wait()
+		require.NoError(t, rotateErr)
+		canceled := cancelErr == nil
+		require.True(t, canceled != rotated, "round %d: exactly one of the cancel (err=%v) and the rotation (%v) may win", i, cancelErr, rotated)
+		after := statusInDB(ctx, t, store, row.ExecutionID)
+		if canceled {
+			cancelWins++
+			assert.Equal(t, config.StatusCanceled, after.Status)
+			continue
+		}
+		rotationWins++
+		// The snapshot check may already have seen the new token (invalid) or
+		// the pinned cancel may have lost to it (changed); both refuse.
+		assert.True(t, errors.Is(cancelErr, ErrInvalidApprovalToken) || errors.Is(cancelErr, config.ErrApprovalTokenChanged), "round %d: %v", i, cancelErr)
+		assert.Equal(t, "notified", after.Status)
+		assert.True(t, config.ApprovalTokenMatches(after.ApprovalToken, rotNewToken))
+	}
+	t.Logf("cancel won %d, rotation won %d of %d rounds", cancelWins, rotationWins, rounds)
+	assert.Positive(t, cancelWins, "the race never let the cancel go first")
+	assert.Positive(t, rotationWins, "the race never let the rotation go first")
+}

From 756bf2416bff412f75797fab51fc53b3ab3dff02 Mon Sep 17 00:00:00 2001
From: Astrid Gealer <astrid@gealer.email>
Date: Thu, 22 Oct 2026 02:23:58 +0100
Subject: [PATCH 3/3] sec(api): compare the revoke token before reporting
 expiry or a closed window

validateRevokeToken checked the link's expiry and the revocation window
before comparing the token hash, so a wrong token on a lapsed row got
"revocation link has expired" (409) or "revocation window has closed" (403)
instead of "invalid revocation token", telling a token guesser the link
exists and has lapsed (issue #1190). The hash is now compared first, as the
approve path's ValidateApprovalToken does.

TestHandler_revokePurchase_ExpiredToken stored the raw token instead of its
hash, so its "matching" token never matched; it passed only because expiry
was checked first. The fixture now stores the hash.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 internal/api/handler_purchases.go             | 31 ++++++++--------
 internal/api/handler_purchases_test.go        |  6 ++-
 .../handler_purchases_token_errors_test.go    | 37 +++++++++++++++++++
 3 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go
index 0b0d09fb1..19f828d19 100644
--- a/internal/api/handler_purchases.go
+++ b/internal/api/handler_purchases.go
@@ -1916,13 +1916,23 @@ func checkRevokableStatus(execution *config.PurchaseExecution) error {
 }
 
 // validateRevokeToken checks that the execution carries a non-empty
-// ApprovalToken, that the token has not expired, that the revocation window
-// has not closed (config.RevocationWindow after CompletedAt), and that the
-// token matches using constant-time comparison. Pre-migration rows without an
-// ApprovalTokenExpiresAt pass the expiry check (legacy backward compat).
+// ApprovalToken that matches the supplied token (constant-time comparison),
+// then that the token has not expired and that the revocation window has not
+// closed (config.RevocationWindow after CompletedAt). Pre-migration rows
+// without an ApprovalTokenExpiresAt pass the expiry check (legacy backward
+// compat).
+//
+// The token is compared first, as on the approve path (ValidateApprovalToken):
+// a wrong token must get the same "invalid" answer whatever the row's expiry or
+// window, or the expiry/window message tells a token guesser that the link
+// exists and has lapsed (issue #1190).
 // Extracted from revokeViaEmailToken to keep that function under the cyclomatic limit.
 func validateRevokeToken(execution *config.PurchaseExecution, token string) error {
-	if execution.ApprovalToken == "" {
+	// execution.ApprovalToken is the SHA-256 hex digest stored at rest
+	// (issue #103), not the raw secret; config.ApprovalTokenMatches hashes
+	// the supplied token and compares digests in constant time, and is false
+	// for an empty stored or supplied token.
+	if !config.ApprovalTokenMatches(execution.ApprovalToken, token) {
 		return NewClientError(403, "invalid revocation token")
 	}
 	if execution.ApprovalTokenExpiresAt != nil && time.Now().After(*execution.ApprovalTokenExpiresAt) {
@@ -1932,16 +1942,7 @@ func validateRevokeToken(execution *config.PurchaseExecution, token string) erro
 	// Enforce the 24-hour revocation window (issue #291 Finding #6).
 	// Use CompletedAt as the reference timestamp; fall back to ExecutedAt
 	// for direct-execute rows which may not have a CompletedAt yet.
-	if windowErr := checkRevocationWindow(execution); windowErr != nil {
-		return windowErr
-	}
-	// execution.ApprovalToken is the SHA-256 hex digest stored at rest
-	// (issue #103), not the raw secret; config.ApprovalTokenMatches hashes
-	// the supplied token and compares digests in constant time.
-	if !config.ApprovalTokenMatches(execution.ApprovalToken, token) {
-		return NewClientError(403, "invalid revocation token")
-	}
-	return nil
+	return checkRevocationWindow(execution)
 }
 
 // checkRevocationWindow returns a 403 ClientError when the 24-hour revocation
diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go
index 2d6e7346c..e19ea8d9c 100644
--- a/internal/api/handler_purchases_test.go
+++ b/internal/api/handler_purchases_test.go
@@ -5908,8 +5908,10 @@ func TestHandler_revokePurchase_ExpiredToken(t *testing.T) {
 	past := time.Now().Add(-1 * time.Hour)
 
 	exec := &config.PurchaseExecution{
-		ExecutionID:            execID,
-		ApprovalToken:          token,
+		ExecutionID: execID,
+		// Stored at rest as the hash (issue #103), so the token really matches
+		// and the expiry is what rejects it (the hash is compared first, issue #1190).
+		ApprovalToken:          config.HashApprovalToken(token),
 		ApprovalTokenExpiresAt: &past,
 		Status:                 "completed",
 		Recommendations: []config.RecommendationRecord{
diff --git a/internal/api/handler_purchases_token_errors_test.go b/internal/api/handler_purchases_token_errors_test.go
index 050f02920..7e169dbeb 100644
--- a/internal/api/handler_purchases_token_errors_test.go
+++ b/internal/api/handler_purchases_token_errors_test.go
@@ -166,3 +166,40 @@ func TestHandleRequest_TokenActions_TokenRotatedBeforeCASIs403(t *testing.T) {
 		})
 	}
 }
+
+// Issue #1190: the revoke endpoint compares the token before the link's expiry
+// and the revocation window, as approve does, so a wrong token gets the same
+// 403 "invalid revocation token" whether the row's link is live, expired or
+// past its window; only the right token learns that the link has lapsed.
+func TestHandleRequest_RevokeViaEmail_WrongTokenOnLapsedRowIsInvalid(t *testing.T) {
+	past := time.Now().Add(-time.Hour)
+	future := time.Now().Add(time.Hour)
+	longAgo := time.Now().Add(-2 * config.RevocationWindow)
+	hash := config.HashApprovalToken(tokenErrRawToken)
+	cases := []struct {
+		name      string
+		exec      config.PurchaseExecution
+		token     string
+		want      int
+		wantInMsg string
+	}{
+		{"wrong token on an expired link", config.PurchaseExecution{Status: "completed", ApprovalToken: hash, ApprovalTokenExpiresAt: &past}, "wrong", 403, "invalid revocation token"},
+		{"wrong token past the revocation window", config.PurchaseExecution{Status: "completed", ApprovalToken: hash, ApprovalTokenExpiresAt: &future, CompletedAt: &longAgo}, "wrong", 403, "invalid revocation token"},
+		{"right token on an expired link", config.PurchaseExecution{Status: "completed", ApprovalToken: hash, ApprovalTokenExpiresAt: &past}, tokenErrRawToken, 409, "expired"},
+		{"right token past the revocation window", config.PurchaseExecution{Status: "completed", ApprovalToken: hash, ApprovalTokenExpiresAt: &future, CompletedAt: &longAgo}, tokenErrRawToken, 403, "revocation window has closed"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			exec := tc.exec
+			h := tokenErrHandler(t, &exec, nil)
+			resp, err := h.HandleRequest(context.Background(), tokenErrRequest("revoke", tc.token))
+			require.NoError(t, err)
+			assert.Equal(t, tc.want, resp.StatusCode, resp.Body)
+			assert.Contains(t, resp.Body, tc.wantInMsg)
+			if tc.token != tokenErrRawToken {
+				assert.NotContains(t, resp.Body, "expired")
+				assert.NotContains(t, resp.Body, "window")
+			}
+		})
+	}
+}