fix(api): answer a missing RI exchange like a wrong token on approve and reject

internal/api/coverage_extras_test.go

@@ -318,8 +318,12 @@ func TestHandler_rejectRIExchange_RecordNotFound(t *testing.T) {
 
 	h := &Handler{config: mockStore}
 	_, err := h.rejectRIExchange(ctx, "11111111-1111-1111-1111-111111111111", "tok")
-	assert.Error(t, err)
-	assert.Contains(t, err.Error(), "not found")
+	// A missing record answers like a wrong token (issue #601).
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "want ClientError, got %v", err)
+	assert.Equal(t, 403, ce.code)
+	assert.Equal(t, "invalid rejection token", ce.message)
 }
 
 func TestHandler_rejectRIExchange_WrongToken(t *testing.T) {

internal/api/handler_ri_exchange.go

@@ -2546,9 +2546,9 @@ func (h *Handler) fetchAndAuthorizeRIExchange(ctx context.Context, session *Sess
 		return nil, errNotFound
 	}
 
-	record, err := h.config.GetRIExchangeRecord(ctx, id)
+	record, err := h.getRIExchangeRecord(ctx, id)
 	if err != nil {
-		return nil, fmt.Errorf("failed to look up exchange record: %w", err)
+		return nil, err
 	}
 	if record == nil {
 		return nil, NewClientError(404, "exchange record not found")
@@ -2713,6 +2713,22 @@ func (h *Handler) requireDifferentRIExchangeApprover(ctx context.Context, sessio
 	return NewClientError(500, "4-eyes approval mode is enabled but no session could be resolved; sign in before approving")
 }
 
+// getRIExchangeRecord loads an exchange record for the approve and reject
+// routes. A missing record is (nil, nil) whether the store reports it as a
+// nil record or as a wrapped config.ErrNotFound (PostgresStore), so callers
+// answer it as a client error and not a 500 (issue #601). Any other store
+// error stays a server error.
+func (h *Handler) getRIExchangeRecord(ctx context.Context, id string) (*config.RIExchangeRecord, error) {
+	record, err := h.config.GetRIExchangeRecord(ctx, id)
+	if errors.Is(err, config.ErrNotFound) {
+		return nil, nil
+	}
+	if err != nil {
+		return nil, fmt.Errorf("failed to look up exchange record: %w", err)
+	}
+	return record, nil
+}
+
 // validateExchangeApproval validates ID, token, and record state for an exchange approval.
 func (h *Handler) validateExchangeApproval(ctx context.Context, id, token string) (*config.RIExchangeRecord, error) {
 	if err := validateUUID(id); err != nil {
@@ -2722,14 +2738,10 @@ func (h *Handler) validateExchangeApproval(ctx context.Context, id, token string
 		return nil, NewClientError(400, "approval token is required")
 	}
 
-	record, err := h.config.GetRIExchangeRecord(ctx, id)
+	record, err := h.getRIExchangeRecord(ctx, id)
 	if err != nil {
-		return nil, fmt.Errorf("failed to look up exchange record: %w", err)
-	}
-	if record == nil {
-		return nil, NewClientError(404, "exchange record not found")
+		return nil, err
 	}
-
 	if err := checkExchangeToken(record, token, "approval"); err != nil {
 		return nil, err
 	}
@@ -2743,20 +2755,23 @@ const (
 )
 
 // checkExchangeToken applies the emailed-token status mapping shared with the
-// purchase approve, cancel and revoke links (issue #587): a record without a
-// token, or a token that does not match, is 403, and a matching token on an
-// exchange past its expires_at is 410. The match is checked first, so only
-// the token holder learns that the exchange expired. kind is "approval" or
-// "rejection" and only shapes the 403 text.
+// purchase approve, cancel and revoke links (issue #587): a token that does
+// not match is 403, and a matching token on an exchange past its expires_at
+// is 410. The match is checked first, so only the token holder learns that
+// the exchange expired. kind is "approval" or "rejection" and only shapes the
+// 403 text.
+//
+// A missing record (nil) and a record without a token get the same 403 as a
+// wrong token (issue #601): the token alone authorizes these routes, so they
+// are open to callers with no session, and a distinct answer would tell such
+// a caller which exchange ids exist.
 //
 // record.ApprovalToken is the SHA-256 hex digest stored at rest (issue #103);
 // config.ApprovalTokenMatches hashes the supplied token and compares digests
-// in constant time, as everywhere else that validates one of these tokens.
+// in constant time, as everywhere else that validates one of these tokens,
+// and never matches an empty stored digest.
 func checkExchangeToken(record *config.RIExchangeRecord, token, kind string) error {
-	if record.ApprovalToken == "" {
-		return NewClientError(403, "this exchange record does not support "+kind)
-	}
-	if !config.ApprovalTokenMatches(record.ApprovalToken, token) {
+	if record == nil || !config.ApprovalTokenMatches(record.ApprovalToken, token) {
 		return NewClientError(403, "invalid "+kind+" token")
 	}
 	if record.ExpiresAt != nil && time.Now().After(*record.ExpiresAt) {
@@ -2933,14 +2948,10 @@ func (h *Handler) rejectRIExchange(ctx context.Context, id, token string) (any,
 		return nil, NewClientError(400, "rejection token is required")
 	}
 
-	record, err := h.config.GetRIExchangeRecord(ctx, id)
+	record, err := h.getRIExchangeRecord(ctx, id)
 	if err != nil {
-		return nil, fmt.Errorf("failed to look up exchange record: %w", err)
-	}
-	if record == nil {
-		return nil, NewClientError(404, "exchange record not found")
+		return nil, err
 	}
-
 	if tokenErr := checkExchangeToken(record, token, "rejection"); tokenErr != nil {
 		return nil, tokenErr
 	}

internal/api/handler_ri_exchange_test.go

@@ -893,7 +893,8 @@ func TestRejectRIExchange_EmptyStoredToken(t *testing.T) {
 	ce, ok := IsClientError(err)
 	assert.True(t, ok)
 	assert.Equal(t, 403, ce.code)
-	assert.Contains(t, err.Error(), "does not support rejection")
+	// Same answer as a wrong token or a missing record (issue #601).
+	assert.Contains(t, err.Error(), "invalid rejection token")
 
 	mockStore.AssertExpectations(t)
 }

internal/api/openapi.yaml

@@ -2483,8 +2483,10 @@ paths:
         present; a CSRF rejection never falls through. The token is read
         from the query string only. This route
         never returns 401: a request with no token and no usable session
-        fails CSRF validation with 403. A missing record (404) and a wrong
-        token (403) are distinguishable for unauthenticated callers.
+        fails CSRF validation with 403. The token alone authorizes, so the
+        route is open to callers without a session; token mode answers an
+        unknown exchange id with the same 403 as a wrong token, so it does
+        not reveal which ids exist.
       security:
         - bearerAuth: []
         - {}
@@ -2513,15 +2515,22 @@ paths:
           description: >
             CSRF validation failed (including a request with neither a token
             nor a valid session), the session lacks approve permission, the
-            token is invalid, the record does not support approval, or, when a
-            session exists, 4-eyes mode rejects the approver (for example the
-            creator approving their own exchange).
+            token is invalid, or, when a session exists, 4-eyes mode rejects
+            the approver (for example the creator approving their own
+            exchange). In token mode an unknown exchange id, an exchange with
+            no approval token and a wrong token get the same 403 and body.
           content:
             application/json:
               schema:
                 $ref: '#/components/schemas/Error'
         '404':
-          $ref: '#/components/responses/NotFound'
+          description: >
+            Session mode only, the exchange does not exist. Token mode answers
+            an unknown id with 403, like a wrong token.
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
         '409':
           description: >
             The exchange is no longer pending (already processed, or canceled
@@ -2561,9 +2570,9 @@ paths:
       summary: Reject a pending RI exchange (token-based, no session required)
       description: >
         Authorized by the rejection token in the query string alone. A session
-        is ignored and no CSRF check runs. A missing token is 400; a missing
-        record (404) and a wrong token (403) are distinguishable for
-        unauthenticated callers.
+        is ignored and no CSRF check runs. A missing token is 400. An unknown
+        exchange id gets the same 403 and body as a wrong token, so the route
+        does not reveal which ids exist.
       security: []
       parameters:
         - name: token
@@ -2581,13 +2590,13 @@ paths:
         '400':
           $ref: '#/components/responses/BadRequest'
         '403':
-          description: The rejection token is invalid, or the record does not support rejection.
+          description: >
+            The rejection token is invalid. An unknown exchange id and an
+            exchange with no token get the same answer.
           content:
             application/json:
               schema:
                 $ref: '#/components/schemas/Error'
-        '404':
-          $ref: '#/components/responses/NotFound'
         '409':
           description: >
             The exchange is no longer pending (already processed or canceled),

internal/api/ri_exchange_approve_reject_status_test.go

@@ -29,6 +29,9 @@ const (
 	riRecPending riStatusRecord = iota
 	riRecNoToken
 	riRecMissing
+	// riRecMissingNotFound is missing as PostgresStore reports it: a wrapped
+	// config.ErrNotFound instead of a nil record (issue #601).
+	riRecMissingNotFound
 	riRecAlreadyProcessed
 	// riRecExpired is pending but past its expires_at. Its transition is
 	// mocked to succeed, so a 410 proves the token check answered before
@@ -72,7 +75,7 @@ type riStatusOpts struct {
 
 func riStatusHandler(rec riStatusRecord, o riStatusOpts) *Handler {
 	var record *config.RIExchangeRecord
-	if rec != riRecMissing {
+	if rec != riRecMissing && rec != riRecMissingNotFound {
 		record = &config.RIExchangeRecord{
 			ID: riStatusID, Status: "pending", SourceRIIDs: []string{"ri-1"}, PaymentDue: "0",
 			ApprovalToken: config.HashApprovalToken(riStatusRawToken),
@@ -93,9 +96,13 @@ func riStatusHandler(rec riStatusRecord, o riStatusOpts) *Handler {
 		}
 	}
 	store := new(MockConfigStore)
-	if o.dbError {
+	switch {
+	case o.dbError:
 		store.On("GetRIExchangeRecord", mock.Anything, riStatusID).Return(nil, errors.New("db down")).Maybe()
-	} else {
+	case rec == riRecMissingNotFound:
+		store.On("GetRIExchangeRecord", mock.Anything, riStatusID).
+			Return(nil, fmt.Errorf("ri exchange record %s: %w", riStatusID, config.ErrNotFound)).Maybe()
+	default:
 		store.On("GetRIExchangeRecord", mock.Anything, riStatusID).Return(record, nil).Maybe()
 	}
 	transitioned, transitionErr := riStatusTransition(rec)
@@ -209,7 +216,7 @@ func TestHandleRequest_ApproveRIExchange_StatusCodes(t *testing.T) {
 		{name: "403 session without CSRF and no token", rec: riRecPending, sess: riSessNoCSRF, want: 403, wantError: "CSRF validation failed"},
 		{name: "403 session without CSRF does not fall through to a valid token", rec: riRecPending, sess: riSessNoCSRF, token: riStatusRawToken, want: 403, wantError: "CSRF validation failed"},
 		{name: "403 wrong token without session", rec: riRecPending, sess: riSessNone, token: "wrong", want: 403, wantError: "invalid approval token"},
-		{name: "403 record without approval token", rec: riRecNoToken, sess: riSessNone, token: riStatusRawToken, want: 403, wantError: "this exchange record does not support approval"},
+		{name: "403 record without approval token", rec: riRecNoToken, sess: riSessNone, token: riStatusRawToken, want: 403, wantError: "invalid approval token"},
 		{name: "400 invalid UUID", rec: riRecPending, sess: riSessNone, token: riStatusRawToken, id: "not-a-uuid", want: 400, wantError: "invalid ID format: must be a valid UUID"},
 		{name: "403 session without approve right and no token", rec: riRecPending, sess: riSessCSRF, opts: riStatusOpts{noApprove: true}, want: 403, wantError: "permission denied: requires approve-any or approve-own on purchases"},
 		{name: "403 4-eyes creator cannot approve own exchange", rec: riRecPending, sess: riSessCSRF, opts: riStatusOpts{fourEyes: true}, want: 403, wantError: "approval declined: 4-eyes mode requires a different approver than the requester"},
@@ -220,9 +227,13 @@ func TestHandleRequest_ApproveRIExchange_StatusCodes(t *testing.T) {
 		{name: "403 body token is not read", rec: riRecPending, sess: riSessNone, body: `{"token":"` + riStatusRawToken + `"}`, want: 403, wantError: "CSRF validation failed"},
 		{name: "429 rate limited", rec: riRecPending, sess: riSessNone, token: riStatusRawToken, opts: riStatusOpts{rateLimited: true}, want: 429, wantError: "too many requests, please try again later"},
 		{name: "500 record lookup fails", rec: riRecPending, sess: riSessNone, token: riStatusRawToken, opts: riStatusOpts{dbError: true}, want: 500},
-		{name: "404 missing record with token", rec: riRecMissing, sess: riSessNone, token: riStatusRawToken, want: 404, wantError: "exchange record not found"},
-		{name: "404 missing record with wrong token", rec: riRecMissing, sess: riSessNone, token: "wrong", want: 404, wantError: "exchange record not found"},
+		// Issue #601: in token mode a missing record answers like a wrong
+		// token, so the route does not reveal which ids exist.
+		{name: "403 missing record with token", rec: riRecMissing, sess: riSessNone, token: riStatusRawToken, want: 403, wantError: "invalid approval token"},
+		{name: "403 missing record with wrong token", rec: riRecMissing, sess: riSessNone, token: "wrong", want: 403, wantError: "invalid approval token"},
+		{name: "403 missing record reported as ErrNotFound with token", rec: riRecMissingNotFound, sess: riSessNone, token: riStatusRawToken, want: 403, wantError: "invalid approval token"},
 		{name: "404 missing record with session", rec: riRecMissing, sess: riSessCSRF, want: 404, wantError: "exchange record not found"},
+		{name: "404 missing record reported as ErrNotFound with session", rec: riRecMissingNotFound, sess: riSessCSRF, want: 404, wantError: "exchange record not found"},
 		{name: "409 token approve of an exchange no longer pending", rec: riRecAlreadyProcessed, sess: riSessNone, token: riStatusRawToken, want: 409, wantError: "exchange already processed or was canceled by a newer analysis run"},
 		// Issue #587: the same invalid/expired/wrong-status mapping as the
 		// purchase approve, cancel and revoke links (403/410/409).
@@ -250,9 +261,11 @@ func TestHandleRequest_RejectRIExchange_StatusCodes(t *testing.T) {
 		{name: "429 rate limited", rec: riRecPending, sess: riSessNone, token: riStatusRawToken, opts: riStatusOpts{rateLimited: true}, want: 429, wantError: "too many requests, please try again later"},
 		{name: "500 record lookup fails", rec: riRecPending, sess: riSessNone, token: riStatusRawToken, opts: riStatusOpts{dbError: true}, want: 500},
 		{name: "403 wrong token", rec: riRecPending, sess: riSessNone, token: "wrong", want: 403, wantError: "invalid rejection token"},
-		{name: "403 record without approval token", rec: riRecNoToken, sess: riSessNone, token: riStatusRawToken, want: 403, wantError: "this exchange record does not support rejection"},
-		{name: "404 missing record with token", rec: riRecMissing, sess: riSessNone, token: riStatusRawToken, want: 404, wantError: "exchange record not found"},
-		{name: "404 missing record with wrong token", rec: riRecMissing, sess: riSessNone, token: "wrong", want: 404, wantError: "exchange record not found"},
+		{name: "403 record without approval token", rec: riRecNoToken, sess: riSessNone, token: riStatusRawToken, want: 403, wantError: "invalid rejection token"},
+		// Issue #601: a missing record answers like a wrong token.
+		{name: "403 missing record with token", rec: riRecMissing, sess: riSessNone, token: riStatusRawToken, want: 403, wantError: "invalid rejection token"},
+		{name: "403 missing record with wrong token", rec: riRecMissing, sess: riSessNone, token: "wrong", want: 403, wantError: "invalid rejection token"},
+		{name: "403 missing record reported as ErrNotFound with token", rec: riRecMissingNotFound, sess: riSessNone, token: riStatusRawToken, want: 403, wantError: "invalid rejection token"},
 		{name: "409 exchange no longer pending", rec: riRecAlreadyProcessed, sess: riSessNone, token: riStatusRawToken, want: 409, wantError: "exchange already processed or was canceled"},
 		// Issue #587: 403 invalid, 410 expired, 409 wrong status.
 		{name: "410 expired exchange", rec: riRecExpired, sess: riSessNone, token: riStatusRawToken, want: 410, wantError: "exchange has expired"},
@@ -262,3 +275,58 @@ func TestHandleRequest_RejectRIExchange_StatusCodes(t *testing.T) {
 		{name: "500 transition store failure", rec: riRecTransitionFails, sess: riSessNone, token: riStatusRawToken, want: 500},
 	})
 }
+
+// TestHandleRequest_RIExchangeTokenRoutes_MissingAnswersLikeWrongToken is the
+// issue #601 regression: a caller that does not hold the emailed token gets
+// the same status, headers and body for a missing exchange id as for an
+// existing one, so approve and reject cannot be used to learn which ids
+// exist. A 401 before the lookup (the purchase routes' fix, #435) does not
+// fit here: the token alone authorizes these routes, with no session.
+func TestHandleRequest_RIExchangeTokenRoutes_MissingAnswersLikeWrongToken(t *testing.T) {
+	type probe struct {
+		name  string
+		rec   riStatusRecord
+		token string
+	}
+	// The baseline is a missing record as the mocks report it; every other
+	// probe must answer exactly like it.
+	baseline := probe{"missing record", riRecMissing, "wrong"}
+	probes := []probe{
+		{"missing record reported as ErrNotFound", riRecMissingNotFound, "wrong"},
+		{"missing record reported as ErrNotFound with the real token's value", riRecMissingNotFound, riStatusRawToken},
+		{"pending record with a wrong token", riRecPending, "wrong"},
+		{"record without a token", riRecNoToken, "wrong"},
+		{"expired record with a wrong token", riRecExpired, "wrong"},
+	}
+	callers := []struct {
+		name string
+		sess riStatusSession
+		opts riStatusOpts
+	}{
+		{"no session", riSessNone, riStatusOpts{}},
+		{"invalid session", riSessInvalid, riStatusOpts{}},
+		{"session without approve permission", riSessCSRF, riStatusOpts{noApprove: true}},
+	}
+	for _, action := range []string{"approve", "reject"} {
+		for _, caller := range callers {
+			do := func(p probe) *events.LambdaFunctionURLResponse {
+				resp, err := riStatusHandler(p.rec, caller.opts).HandleRequest(context.Background(), riStatusRequest(action, caller.sess, p.token))
+				require.NoError(t, err)
+				return resp
+			}
+			want := do(baseline)
+			t.Run(action+"/"+caller.name+"/answers 403 invalid token", func(t *testing.T) {
+				assert.Equal(t, 403, want.StatusCode, want.Body)
+				assert.Contains(t, want.Body, "invalid "+map[string]string{"approve": "approval", "reject": "rejection"}[action]+" token")
+			})
+			for _, p := range probes {
+				t.Run(action+"/"+caller.name+"/"+p.name, func(t *testing.T) {
+					got := do(p)
+					assert.Equal(t, want.StatusCode, got.StatusCode, got.Body)
+					assert.Equal(t, want.Body, got.Body)
+					assert.Equal(t, want.Headers, got.Headers)
+				})
+			}
+		}
+	}
+}