diff --git a/.changeset/fix-access-control-silent-error.md b/.changeset/fix-access-control-silent-error.md new file mode 100644 index 00000000000..2b5c628ce9e --- /dev/null +++ b/.changeset/fix-access-control-silent-error.md @@ -0,0 +1,5 @@ +--- +"@keystone-6/core": patch +--- + +Fixed an issue where database errors (connection timeouts, malformed queries, etc.) were swallowed in unique item exists checks at the access control layer. diff --git a/.changeset/fix-validation-feedback-loss.md b/.changeset/fix-validation-feedback-loss.md new file mode 100644 index 00000000000..3a5c49ba694 --- /dev/null +++ b/.changeset/fix-validation-feedback-loss.md @@ -0,0 +1,5 @@ +--- +"@keystone-6/core": patch +--- + +Fixed an issue where field-level validation messages were lost if a list-level validation hook threw an exception. Field validation messages are now prioritized and preserved in list-level hook crashes. diff --git a/packages/core/src/lib/core/access-control.ts b/packages/core/src/lib/core/access-control.ts index 88a488123c8..c7718f37d34 100644 --- a/packages/core/src/lib/core/access-control.ts +++ b/packages/core/src/lib/core/access-control.ts @@ -216,6 +216,7 @@ export async function enforceListLevelAccessControl( listKey: list.listKey, context, item, + inputData, }) } } catch (error: any) { @@ -428,7 +429,15 @@ export async function checkUniqueItemExists( try { const item = await context.db[foreignList.listKey].findOne({ where: uniqueInput }) if (item !== null) return uniqueWhere - } catch (err) {} + } catch (err: any) { + // If it's an access denied error from context.db, we swallow it and throw our own + // to keep the error message consistent with "item may not exist". + // But if it's a real database error (e.g. connection, timeout, malformed query), + // we MUST rethrow it so the developer knows what happened. + if (err?.extensions?.code !== 'KS_ACCESS_DENIED') { + throw err + } + } throw accessDeniedError(cannotForItem(operation, foreignList)) } diff --git a/packages/core/src/lib/core/hooks.ts b/packages/core/src/lib/core/hooks.ts index b6e90d85bf4..de06ac932a4 100644 --- a/packages/core/src/lib/core/hooks.ts +++ b/packages/core/src/lib/core/hooks.ts @@ -1,15 +1,12 @@ import { extensionError, validationFailureError } from './graphql-errors' -import { type InitialisedList } from './initialise-lists' +import type { InitialisedList } from './initialise-lists' -export async function validate({ +async function validate({ list, hookArgs, }: { list: InitialisedList - hookArgs: Omit< - Parameters[0], - 'addValidationError' - > + hookArgs: any }) { const messages: string[] = [] const fieldsErrors: { error: Error; tag: string }[] = [] @@ -32,13 +29,13 @@ export async function validate({ resolvedFieldData: hookArgs.resolvedData?.[fieldKey], } as never) // TODO: FIXME } catch (error: any) { - fieldsErrors.push({ error, tag: `${list.listKey}.${fieldKey}.hooks.validateInput` }) + fieldsErrors.push({ error, tag: `${list.listKey}.${fieldKey}.hooks.validate` }) } }) ) if (fieldsErrors.length) { - throw extensionError('validateInput', fieldsErrors) + throw extensionError('validate', fieldsErrors) } // list validation hooks @@ -46,24 +43,26 @@ export async function validate({ const addValidationError = (msg: string) => void messages.push(`${list.listKey}: ${msg}`) const hook = list.hooks.validate[operation] + let listHookError: any try { await hook({ ...hookArgs, addValidationError } as never) // TODO: FIXME } catch (error: any) { - throw extensionError('validateInput', [{ error, tag: `${list.listKey}.hooks.validateInput` }]) + listHookError = error } if (messages.length) { throw validationFailureError(messages) } + + if (listHookError) { + throw extensionError('validate', [{ error: listHookError, tag: `${list.listKey}.hooks.validate` }]) + } } } -export async function runSideEffectOnlyHook< +async function runSideEffectOnlyHook< HookName extends 'beforeOperation' | 'afterOperation', - Args extends Parameters< - NonNullable - >[0], ->(list: InitialisedList, hookName: HookName, args: Args) { +>(list: InitialisedList, hookName: HookName, args: any) { const { operation } = args let shouldRunFieldLevelHook: (fieldKey: string) => boolean @@ -109,3 +108,5 @@ export async function runSideEffectOnlyHook< throw extensionError(hookName, [{ error, tag: `${list.listKey}.hooks.${hookName}` }]) } } + +export { validate, runSideEffectOnlyHook } diff --git a/packages/core/src/types/config/hooks.ts b/packages/core/src/types/config/hooks.ts index 2263fb7c717..37836ea06d2 100644 --- a/packages/core/src/types/config/hooks.ts +++ b/packages/core/src/types/config/hooks.ts @@ -65,7 +65,7 @@ export type ListHooks = { } /** - * Used to **cause side effects** before a create, update, or delete operation once all validateInput hooks have resolved + * Used to **cause side effects** before a create, update, or delete operation once all validate hooks have resolved */ beforeOperation?: | BeforeOperationListHook @@ -135,7 +135,7 @@ export type FieldHooks< } /** - * Used to **cause side effects** before a create, update, or delete operation once all validateInput hooks have resolved + * Used to **cause side effects** before a create, update, or delete operation once all validate hooks have resolved */ beforeOperation?: | BeforeOperationFieldHook