fix(api): validate the emailed approval token on the delayed-approval path

internal/api/handler_purchases.go

@@ -695,6 +695,11 @@ func (h *Handler) approveViaToken(ctx context.Context, req *events.LambdaFunctio
 		return nil, fmt.Errorf("failed to read global config for purchase delay check: %w", cfgErr)
 	}
 	if globalCfg.GetPurchaseDelay() > 0 {
+		// The delayed branch bypasses ApproveExecution, which is where the
+		// token is otherwise validated (issue #586).
+		if err := purchase.ValidateApprovalToken(execution, token); err != nil {
+			return nil, tokenActionError(err)
+		}
 		return h.approveWithDelay(ctx, execution, globalCfg.GetPurchaseDelay(), actor, nil)
 	}
 	// ApproveExecution now runs the purchase synchronously inside the

internal/api/handler_purchases_delay_token_test.go

@@ -0,0 +1,96 @@
+package api
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+)
+
+// The delayed-approval branch of the email-link approve must validate the
+// token exactly like the immediate branch (issue #586), reusing the harness
+// from handler_purchases_token_errors_test.go.
+
+func delayTokenHandler(t *testing.T, exec *config.PurchaseExecution, delayHours int) *Handler {
+	t.Helper()
+	h := tokenErrHandler(t, exec, nil)
+	store := h.config.(*MockConfigStore)
+	kept := store.ExpectedCalls[:0]
+	for _, c := range store.ExpectedCalls {
+		if c.Method != "GetGlobalConfig" {
+			kept = append(kept, c)
+		}
+	}
+	store.ExpectedCalls = kept
+	approver := "admin@example.com"
+	store.On("GetGlobalConfig", mock.Anything).
+		Return(&config.GlobalConfig{NotificationEmail: &approver, PurchaseDelayHours: delayHours}, nil)
+	return h
+}
+
+func TestHandleRequest_ApproveViaToken_DelayConfigured_ValidatesToken(t *testing.T) {
+	past := time.Now().Add(-time.Hour)
+	future := time.Now().Add(time.Hour)
+	hash := config.HashApprovalToken(tokenErrRawToken)
+
+	cases := []struct {
+		name  string
+		exec  config.PurchaseExecution
+		token string
+		want  int
+	}{
+		{"wrong token", config.PurchaseExecution{Status: "pending", ApprovalToken: hash, ApprovalTokenExpiresAt: &future}, "WRONG", 403},
+		{"expired token", config.PurchaseExecution{Status: "pending", ApprovalToken: hash, ApprovalTokenExpiresAt: &past}, tokenErrRawToken, 410},
+		{"execution without a stored token", config.PurchaseExecution{Status: "pending", ApprovalTokenExpiresAt: &future}, tokenErrRawToken, 403},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			exec := tc.exec
+			h := delayTokenHandler(t, &exec, 1)
+			store := h.config.(*MockConfigStore)
+			// Registered so a regression answers 200 instead of panicking on an unmocked call.
+			store.On("TransitionExecutionStatus", mock.Anything, tokenErrExecID, mock.Anything, "scheduled", mock.Anything).
+				Return(&config.PurchaseExecution{ExecutionID: tokenErrExecID, Status: "scheduled"}, nil).Maybe()
+			store.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil).Maybe()
+			resp, err := h.HandleRequest(context.Background(), tokenErrRequest("approve", tc.token))
+			require.NoError(t, err)
+			assert.Equal(t, tc.want, resp.StatusCode, resp.Body)
+			store.AssertNotCalled(t, "TransitionExecutionStatus",
+				mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+		})
+	}
+}
+
+func TestHandleRequest_ApproveViaToken_DelayConfigured_ValidTokenSchedules(t *testing.T) {
+	future := time.Now().Add(time.Hour)
+	exec := &config.PurchaseExecution{Status: "pending", ApprovalToken: config.HashApprovalToken(tokenErrRawToken), ApprovalTokenExpiresAt: &future}
+	h := delayTokenHandler(t, exec, 1)
+	store := h.config.(*MockConfigStore)
+	scheduled := &config.PurchaseExecution{ExecutionID: tokenErrExecID, Status: "scheduled"}
+	store.On("TransitionExecutionStatus", mock.Anything, tokenErrExecID, []string{"pending", "notified"}, "scheduled", mock.Anything).
+		Return(scheduled, nil)
+	store.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil)
+
+	resp, err := h.HandleRequest(context.Background(), tokenErrRequest("approve", tokenErrRawToken))
+	require.NoError(t, err)
+	assert.Equal(t, 200, resp.StatusCode, resp.Body)
+	assert.Contains(t, resp.Body, "scheduled")
+}
+
+func TestHandleRequest_ApproveViaToken_DelayConfigured_ReplayOfScheduledIs409(t *testing.T) {
+	future := time.Now().Add(time.Hour)
+	exec := &config.PurchaseExecution{Status: "scheduled", ApprovalToken: config.HashApprovalToken(tokenErrRawToken), ApprovalTokenExpiresAt: &future}
+	h := delayTokenHandler(t, exec, 1)
+	store := h.config.(*MockConfigStore)
+	store.On("TransitionExecutionStatus", mock.Anything, tokenErrExecID, []string{"pending", "notified"}, "scheduled", mock.Anything).
+		Return(nil, config.ErrExecutionNotInExpectedStatus)
+
+	resp, err := h.HandleRequest(context.Background(), tokenErrRequest("approve", tokenErrRawToken))
+	require.NoError(t, err)
+	assert.Equal(t, 409, resp.StatusCode, resp.Body)
+}

internal/api/openapi.yaml

@@ -559,10 +559,9 @@ paths:
             exceeds its Constraints, the caller is not the account's contact
             email, no contact email is configured on the execution's
             accounts, 4-eyes mode rejects the approver, or (token mode) the
-            approval token is invalid. Token validation applies on the
-            immediate-approval path only; the delayed-approval path (purchase
-            delay configured) does not validate the token yet (tracked
-            separately).
+            approval token is invalid. Token mode validates the token on
+            both the immediate and the delayed-approval (purchase delay
+            configured) paths.
           content:
             application/json:
               schema:
@@ -588,9 +587,8 @@ paths:
                 $ref: '#/components/schemas/Error'
         '410':
           description: >
-            Token mode only, the approval token has expired (immediate-approval
-            path only; the delayed-approval path does not validate the token yet,
-            tracked separately).
+            Token mode only, the approval token has expired (checked on both
+            the immediate and the delayed-approval paths).
           content:
             application/json:
               schema:

internal/purchase/approvals.go

@@ -58,7 +58,7 @@ func (m *Manager) ApproveExecution(ctx context.Context, executionID, token, acto
 	}
 
 	// Validate token and TTL (Finding #4 + issue #397).
-	if tokErr := validateApprovalToken(execution, token); tokErr != nil {
+	if tokErr := ValidateApprovalToken(execution, token); tokErr != nil {
 		return "", tokErr
 	}
 
@@ -105,17 +105,19 @@ func maskActor(actor string) string {
 	return "****"
 }
 
-// validateApprovalToken checks that the execution carries a non-empty token,
+// ValidateApprovalToken checks that the execution carries a non-empty token,
 // that the supplied token matches the stored one using constant-time comparison
 // (Finding #4 -- prevents timing attacks), and that the token has not expired
 // (issue #397). Legacy rows with a nil ApprovalTokenExpiresAt pass the TTL
 // check for backward compatibility. Extracted from ApproveExecution to keep
-// that function under the gocyclo threshold.
+// that function under the gocyclo threshold. Exported so the HTTP layer's
+// delayed-approval branch (which never calls ApproveExecution) validates the
+// token the same way (issue #586).
 //
 // execution.ApprovalToken is the SHA-256 hex digest stored at rest
 // (issue #103), never the raw secret; config.ApprovalTokenMatches hashes the
 // supplied token and compares digests in constant time.
-func validateApprovalToken(execution *config.PurchaseExecution, token string) error {
+func ValidateApprovalToken(execution *config.PurchaseExecution, token string) error {
 	if execution.ApprovalToken == "" || token == "" {
 		return ErrInvalidApprovalToken
 	}
@@ -563,10 +565,10 @@ func (m *Manager) loadCancelableExecution(ctx context.Context, executionID, toke
 		return nil, fmt.Errorf("failed to get execution: %w", err)
 	}
 	// execution.ApprovalToken is the SHA-256 hex digest stored at rest
-	// (issue #103); validateApprovalToken hashes the supplied token and
+	// (issue #103); ValidateApprovalToken hashes the supplied token and
 	// compares digests in constant time, and enforces the TTL (issue #397;
 	// legacy rows without ApprovalTokenExpiresAt pass).
-	if err := validateApprovalToken(execution, token); err != nil {
+	if err := ValidateApprovalToken(execution, token); err != nil {
 		return nil, err
 	}