Skip to content

Commit c74a40f

Browse files
AlessioGrAlessioGr
authored
fix: tighten up error visibility handling (#14606)
Fixes #14377 ## Non-public errors were returned from delete and update endpoints Previously, non-public errors (`500` errors, or errors without `isPublic: true` and `status`) were returned from the delete and update endpoint. For security reason, this PR excludes the error messages from the response. This aligns bulk endpoints with others (e.g., deleteByID) that already gate error details through `routeError.ts`. ## Loosen up isPublic check when throwing API error `APIError` now infers `isPublic` from the status code unless explicitly provided: - Non-500 (e.g., 4xx): `isPublic` defaults to `true`. - 500 or missing status: `isPublic` defaults to `false`. You can still override by passing a boolean `isPublic` explicitly. Previously, the only way to mark the error as public was: `throw new APIError('error message', 401, null ,false)` which feels unnecessary explicit. Now, you can do this: `throw new APIError('error message', 401)` which previously resulted in `isPublic: false` and now results in `isPublic: true`. ## Stricter error check in routeError `routeError` now uses `isErrorPublic(err, config)` to decide whether to expose the error message. Compared to the old condition (`!config.debug && !err.isPublic && status === 500)`, this is a bit stricter and easier to understand. Errors with `isPublic === false` are now hidden even for non-500 statuses (previously they were shown).
1 parent 337409b commit c74a40f

File tree

16 files changed

+374
-67
lines changed

16 files changed

+374
-67
lines changed

‎packages/payload/src/collections/config/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -749,6 +749,7 @@ export type BulkOperationResult<TSlug extends CollectionSlug, TSelect extends Se
749749
docs: TransformCollectionWithSelect<TSlug, TSelect>[]
750750
errors: {
751751
id: DataFromCollectionSlug<TSlug>['id']
752+
isPublic: boolean
752753
message: string
753754
}[]
754755
}

‎packages/payload/src/collections/endpoints/delete.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,15 @@ export const deleteHandler: PayloadHandler = async (req) => {
5050
)
5151
}
5252

53+
result.errors = result.errors.map((error) =>
54+
error.isPublic
55+
? error
56+
: {
57+
...error,
58+
message: 'Something went wrong.',
59+
},
60+
)
61+
5362
const total = result.docs.length + result.errors.length
5463

5564
const message = req.t('error:unableToDeleteCount', {

‎packages/payload/src/collections/endpoints/update.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,15 @@ export const updateHandler: PayloadHandler = async (req) => {
5656
)
5757
}
5858

59+
result.errors = result.errors.map((error) =>
60+
error.isPublic
61+
? error
62+
: {
63+
...error,
64+
message: 'Something went wrong.',
65+
},
66+
)
67+
5968
const total = result.docs.length + result.errors.length
6069
const message = req.t('error:unableToUpdateCount', {
6170
count: result.errors.length,

‎packages/payload/src/collections/operations/delete.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { appendNonTrashedFilter } from '../../utilities/appendNonTrashedFilter.j
2222
import { checkDocumentLockStatus } from '../../utilities/checkDocumentLockStatus.js'
2323
import { commitTransaction } from '../../utilities/commitTransaction.js'
2424
import { initTransaction } from '../../utilities/initTransaction.js'
25+
import { isErrorPublic } from '../../utilities/isErrorPublic.js'
2526
import { killTransaction } from '../../utilities/killTransaction.js'
2627
import { sanitizeSelect } from '../../utilities/sanitizeSelect.js'
2728
import { deleteCollectionVersions } from '../../versions/deleteCollectionVersions.js'
@@ -138,7 +139,7 @@ export const deleteOperation = async <
138139
where: fullWhere,
139140
})
140141

141-
const errors: { id: number | string; message: string }[] = []
142+
const errors: BulkOperationResult<TSlug, TSelect>['errors'] = []
142143

143144
const promises = docs.map(async (doc) => {
144145
let result
@@ -281,8 +282,11 @@ export const deleteOperation = async <
281282

282283
return result
283284
} catch (error) {
285+
const isPublic = error instanceof Error ? isErrorPublic(error, config) : false
286+
284287
errors.push({
285288
id: doc.id,
289+
isPublic,
286290
message: error instanceof Error ? error.message : 'Unknown error',
287291
})
288292
}

‎packages/payload/src/collections/operations/update.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { unlinkTempFiles } from '../../uploads/unlinkTempFiles.js'
2323
import { appendNonTrashedFilter } from '../../utilities/appendNonTrashedFilter.js'
2424
import { commitTransaction } from '../../utilities/commitTransaction.js'
2525
import { initTransaction } from '../../utilities/initTransaction.js'
26+
import { isErrorPublic } from '../../utilities/isErrorPublic.js'
2627
import { killTransaction } from '../../utilities/killTransaction.js'
2728
import { sanitizeSelect } from '../../utilities/sanitizeSelect.js'
2829
import { buildVersionCollectionFields } from '../../versions/buildCollectionFields.js'
@@ -225,7 +226,7 @@ export const updateOperation = async <
225226
throwOnMissingFile: false,
226227
})
227228

228-
const errors: { id: number | string; message: string }[] = []
229+
const errors: BulkOperationResult<TSlug, TSelect>['errors'] = []
229230

230231
const promises = docs.map(async (docWithLocales) => {
231232
const { id } = docWithLocales
@@ -265,8 +266,11 @@ export const updateOperation = async <
265266

266267
return updatedDoc
267268
} catch (error) {
269+
const isPublic = error instanceof Error ? isErrorPublic(error, config) : false
270+
268271
errors.push({
269272
id,
273+
isPublic,
270274
message: error instanceof Error ? error.message : 'Unknown error',
271275
})
272276
}

‎packages/payload/src/errors/APIError.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,13 @@ export class APIError<
4747
message: string,
4848
status: number = httpStatus.INTERNAL_SERVER_ERROR,
4949
data: TData = null!,
50-
isPublic = false,
50+
isPublic?: boolean,
5151
) {
52-
super(message, status, data, isPublic)
52+
super(
53+
message,
54+
status,
55+
data,
56+
typeof isPublic === 'boolean' ? isPublic : status !== httpStatus.INTERNAL_SERVER_ERROR,
57+
)
5358
}
5459
}

‎packages/payload/src/utilities/isErrorPublic.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { status as httpStatus } from 'http-status'
2+
3+
import type { SanitizedConfig } from '../config/types.js'
4+
5+
type PayloadError = {
6+
isPublic?: boolean
7+
status?: number
8+
} & Error
9+
10+
/**
11+
* Determines if an error should be shown to the user.
12+
*/
13+
export function isErrorPublic(error: Error, config: SanitizedConfig) {
14+
const payloadError = error as PayloadError
15+
16+
if (config.debug) {
17+
return true
18+
}
19+
if (payloadError.isPublic === true) {
20+
return true
21+
}
22+
if (payloadError.isPublic === false) {
23+
return false
24+
}
25+
if (payloadError.status && payloadError.status !== httpStatus.INTERNAL_SERVER_ERROR) {
26+
return true
27+
}
28+
29+
return false
30+
}

‎packages/payload/src/utilities/routeError.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { APIError } from '../errors/APIError.js'
88
import { getPayload } from '../index.js'
99
import { formatErrors } from './formatErrors.js'
1010
import { headersWithCors } from './headersWithCors.js'
11+
import { isErrorPublic } from './isErrorPublic.js'
1112
import { logError } from './logError.js'
1213
import { mergeHeaders } from './mergeHeaders.js'
1314

@@ -68,7 +69,7 @@ export const routeError = async ({
6869

6970
// Internal server errors can contain anything, including potentially sensitive data.
7071
// Therefore, error details will be hidden from the response unless `config.debug` is `true`
71-
if (!config.debug && !err.isPublic && status === httpStatus.INTERNAL_SERVER_ERROR) {
72+
if (!isErrorPublic(err, config)) {
7273
response = formatErrors(new APIError('Something went wrong.'))
7374
}
7475

‎packages/ui/src/elements/Table/index.tsx‎

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -41,26 +41,29 @@ export const Table: React.FC<Props> = ({ appearance, BeforeTable, columns, data
4141
</thead>
4242
<tbody>
4343
{data &&
44-
data?.map((row, rowIndex) => (
45-
<tr
46-
className={`row-${rowIndex + 1}`}
47-
key={
48-
typeof row.id === 'string' || typeof row.id === 'number'
49-
? String(row.id)
50-
: rowIndex
51-
}
52-
>
53-
{activeColumns.map((col, colIndex) => {
54-
const { accessor } = col
44+
data?.map((row, rowIndex) => {
45+
return (
46+
<tr
47+
className={`row-${rowIndex + 1}`}
48+
data-id={row.id}
49+
key={
50+
typeof row.id === 'string' || typeof row.id === 'number'
51+
? String(row.id)
52+
: rowIndex
53+
}
54+
>
55+
{activeColumns.map((col, colIndex) => {
56+
const { accessor } = col
5557

56-
return (
57-
<td className={`cell-${accessor.replace(/\./g, '__')}`} key={colIndex}>
58-
{col.renderedCells[rowIndex]}
59-
</td>
60-
)
61-
})}
62-
</tr>
63-
))}
58+
return (
59+
<td className={`cell-${accessor.replace(/\./g, '__')}`} key={colIndex}>
60+
{col.renderedCells[rowIndex]}
61+
</td>
62+
)
63+
})}
64+
</tr>
65+
)
66+
})}
6467
</tbody>
6568
</table>
6669
</div>

‎test/buildConfigWithDefaults.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,22 +131,24 @@ export async function buildConfigWithDefaults(
131131
],
132132
}),
133133
email: testEmailAdapter,
134-
endpoints: [localAPIEndpoint, reInitEndpoint],
135134
secret: 'TEST_SECRET',
136135
sharp,
137136
telemetry: false,
138137
...testConfig,
138+
endpoints: [localAPIEndpoint, reInitEndpoint, ...(testConfig?.endpoints || [])],
139139
i18n: {
140140
supportedLanguages: {
141141
de,
142142
en,
143143
es,
144+
...(testConfig?.i18n?.supportedLanguages || {}),
144145
},
145146
...(testConfig?.i18n || {}),
146147
},
147148
typescript: {
148149
declare: {
149150
ignoreTSError: true,
151+
...(testConfig?.typescript?.declare || {}),
150152
},
151153
...testConfig?.typescript,
152154
},

0 commit comments

Comments
 (0)