Skip to content

Commit 054c91c

Browse files
authored
sec(purchases): map GetExecutionByID not-found to 404 in getPurchaseDetails (closes #431) (#528)
getPurchaseDetails previously wrapped any GetExecutionByID error as a plain fmt.Errorf, which the error handler converted to a 500. A 500 vs 404 difference let unauthenticated callers probe for execution UUID existence (information disclosure via status code timing/value). Map both error cases (err != nil, nil execution) to NewClientError(404) so the handler is silent about whether a given UUID exists. Update existing not-found tests to assert on the 404 status code. Add TestHandler_getPurchaseDetails_NotFound_IsClientError as a focused regression for this class of bug.
1 parent db5850e commit 054c91c

2 files changed

Lines changed: 45 additions & 2 deletions

File tree

‎internal/api/handler_purchases.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1172,10 +1172,12 @@ func (h *Handler) getPurchaseDetails(ctx context.Context, req *events.LambdaFunc
11721172

11731173
execution, err := h.config.GetExecutionByID(ctx, executionID)
11741174
if err != nil {
1175-
return nil, fmt.Errorf("execution not found: %w", err)
1175+
// Map any DB-level "not found" error to 404 so callers cannot infer
1176+
// whether a UUID exists by observing a 500 vs a 404 (issue #431).
1177+
return nil, NewClientError(404, "execution not found")
11761178
}
11771179
if execution == nil {
1178-
return nil, fmt.Errorf("execution not found: %s", executionID)
1180+
return nil, NewClientError(404, "execution not found")
11791181
}
11801182

11811183
// Scope: reject if the execution's plan isn't accessible to the session.

‎internal/api/handler_purchases_test.go‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1591,6 +1591,10 @@ func TestHandler_getPurchaseDetails_NotFound(t *testing.T) {
15911591
assert.Error(t, err)
15921592
assert.Nil(t, result)
15931593
assert.Contains(t, err.Error(), "execution not found")
1594+
// Regression #431: must be a 404 ClientError, not a 500.
1595+
ce, ok := IsClientError(err)
1596+
require.True(t, ok, "expected a client error")
1597+
assert.Equal(t, 404, ce.code)
15941598
}
15951599

15961600
func TestHandler_getPurchaseDetails_NilExecution(t *testing.T) {
@@ -1618,6 +1622,43 @@ func TestHandler_getPurchaseDetails_NilExecution(t *testing.T) {
16181622
assert.Error(t, err)
16191623
assert.Nil(t, result)
16201624
assert.Contains(t, err.Error(), "execution not found")
1625+
// Regression #431: must be a 404 ClientError, not a 500.
1626+
ce, ok := IsClientError(err)
1627+
require.True(t, ok, "expected a client error")
1628+
assert.Equal(t, 404, ce.code)
1629+
}
1630+
1631+
// TestHandler_getPurchaseDetails_NotFound_IsClientError is a focused
1632+
// regression for issue #431: a DB-level "not found" error from
1633+
// GetExecutionByID must produce a 404 ClientError, never a 500, so that
1634+
// unauthenticated callers cannot infer UUID existence by observing a
1635+
// status-code difference.
1636+
func TestHandler_getPurchaseDetails_NotFound_IsClientError(t *testing.T) {
1637+
ctx := context.Background()
1638+
mockStore := new(MockConfigStore)
1639+
mockAuth := new(MockAuthService)
1640+
1641+
adminSession := &Session{
1642+
UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
1643+
Email: "admin@example.com",
1644+
Role: "admin",
1645+
}
1646+
mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
1647+
1648+
const missingID = "dddddddd-dddd-dddd-dddd-dddddddddddd"
1649+
// Simulate the DB returning a generic error (as pgx does on a missing row).
1650+
mockStore.On("GetExecutionByID", ctx, missingID).Return(nil, errors.New("no rows in result set"))
1651+
1652+
handler := &Handler{config: mockStore, auth: mockAuth}
1653+
req := &events.LambdaFunctionURLRequest{
1654+
Headers: map[string]string{"Authorization": "Bearer admin-token"},
1655+
}
1656+
1657+
_, err := handler.getPurchaseDetails(ctx, req, missingID)
1658+
require.Error(t, err)
1659+
ce, ok := IsClientError(err)
1660+
require.True(t, ok, "getPurchaseDetails must return a ClientError on not-found, not wrap into a 500")
1661+
assert.Equal(t, 404, ce.code, "not-found from GetExecutionByID must produce HTTP 404")
16211662
}
16221663

16231664
func TestHandler_getPurchaseDetails_WithTimestamps(t *testing.T) {

0 commit comments

Comments
 (0)