-
-
Notifications
You must be signed in to change notification settings - Fork 95
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: merge from dev #1054
chore: merge from dev #1054
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -481,7 +481,7 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr | |||||
// Validates the given create payload against Zod schema if any | ||||||
private validateCreateInputSchema(model: string, data: any) { | ||||||
const schema = this.policyUtils.getZodSchema(model, 'create'); | ||||||
if (schema) { | ||||||
if (schema && data) { | ||||||
const parseResult = schema.safeParse(data); | ||||||
if (!parseResult.success) { | ||||||
throw this.policyUtils.deniedByPolicy( | ||||||
|
@@ -514,26 +514,29 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr | |||||
|
||||||
args = this.policyUtils.clone(args); | ||||||
|
||||||
// do static input validation and check if post-create checks are needed | ||||||
// go through create items, statically check input to determine if post-create | ||||||
// check is needed, and also validate zod schema | ||||||
let needPostCreateCheck = false; | ||||||
for (const item of enumerate(args.data)) { | ||||||
const validationResult = this.validateCreateInputSchema(this.model, item); | ||||||
if (validationResult !== item) { | ||||||
this.policyUtils.replace(item, validationResult); | ||||||
} | ||||||
|
||||||
const inputCheck = this.policyUtils.checkInputGuard(this.model, item, 'create'); | ||||||
if (inputCheck === false) { | ||||||
// unconditionally deny | ||||||
throw this.policyUtils.deniedByPolicy( | ||||||
this.model, | ||||||
'create', | ||||||
undefined, | ||||||
CrudFailureReason.ACCESS_POLICY_VIOLATION | ||||||
); | ||||||
} else if (inputCheck === true) { | ||||||
const r = this.validateCreateInputSchema(this.model, item); | ||||||
if (r !== item) { | ||||||
this.policyUtils.replace(item, r); | ||||||
} | ||||||
// unconditionally allow | ||||||
} else if (inputCheck === undefined) { | ||||||
// static policy check is not possible, need to do post-create check | ||||||
needPostCreateCheck = true; | ||||||
break; | ||||||
} | ||||||
} | ||||||
|
||||||
|
@@ -808,7 +811,13 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr | |||||
|
||||||
// check if the update actually writes to this model | ||||||
let thisModelUpdate = false; | ||||||
const updatePayload: any = (args as any).data ?? args; | ||||||
const updatePayload = (args as any).data ?? args; | ||||||
|
||||||
const validatedPayload = this.validateUpdateInputSchema(model, updatePayload); | ||||||
if (validatedPayload !== updatePayload) { | ||||||
this.policyUtils.replace(updatePayload, validatedPayload); | ||||||
} | ||||||
|
||||||
if (updatePayload) { | ||||||
for (const key of Object.keys(updatePayload)) { | ||||||
const field = resolveField(this.modelMeta, model, key); | ||||||
|
@@ -879,6 +888,8 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr | |||||
); | ||||||
} | ||||||
|
||||||
args.data = this.validateUpdateInputSchema(model, args.data); | ||||||
|
||||||
const updateGuard = this.policyUtils.getAuthGuard(db, model, 'update'); | ||||||
if (this.policyUtils.isTrue(updateGuard) || this.policyUtils.isFalse(updateGuard)) { | ||||||
// injects simple auth guard into where clause | ||||||
|
@@ -939,7 +950,10 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr | |||||
await _registerPostUpdateCheck(model, uniqueFilter); | ||||||
|
||||||
// convert upsert to update | ||||||
context.parent.update = { where: args.where, data: args.update }; | ||||||
context.parent.update = { | ||||||
where: args.where, | ||||||
data: this.validateUpdateInputSchema(model, args.update), | ||||||
}; | ||||||
delete context.parent.upsert; | ||||||
|
||||||
// continue visiting the new payload | ||||||
|
@@ -1038,6 +1052,37 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr | |||||
return { result, postWriteChecks }; | ||||||
} | ||||||
|
||||||
// Validates the given update payload against Zod schema if any | ||||||
private validateUpdateInputSchema(model: string, data: any) { | ||||||
const schema = this.policyUtils.getZodSchema(model, 'update'); | ||||||
if (schema && data) { | ||||||
// update payload can contain non-literal fields, like: | ||||||
// { x: { increment: 1 } } | ||||||
// we should only validate literal fields | ||||||
|
||||||
const literalData = Object.entries(data).reduce<any>( | ||||||
(acc, [k, v]) => ({ ...acc, ...(typeof v !== 'object' ? { [k]: v } : {}) }), | ||||||
{} | ||||||
); | ||||||
|
||||||
const parseResult = schema.safeParse(literalData); | ||||||
if (!parseResult.success) { | ||||||
throw this.policyUtils.deniedByPolicy( | ||||||
model, | ||||||
'update', | ||||||
`input failed validation: ${fromZodError(parseResult.error)}`, | ||||||
CrudFailureReason.DATA_VALIDATION_VIOLATION, | ||||||
parseResult.error | ||||||
); | ||||||
} | ||||||
|
||||||
// schema may have transformed field values, use it to overwrite the original data | ||||||
return { ...data, ...parseResult.data }; | ||||||
} else { | ||||||
return data; | ||||||
} | ||||||
} | ||||||
|
||||||
private isUnsafeMutate(model: string, args: any) { | ||||||
if (!args) { | ||||||
return false; | ||||||
|
@@ -1072,6 +1117,8 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr | |||||
args = this.policyUtils.clone(args); | ||||||
this.policyUtils.injectAuthGuardAsWhere(this.prisma, args, this.model, 'update'); | ||||||
|
||||||
args.data = this.validateUpdateInputSchema(this.model, args.data); | ||||||
|
||||||
Comment on lines
+1120
to
+1121
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Applying Zod schema validation to - args.data = this.validateUpdateInputSchema(this.model, args.data);
+ // Consider batching or optimizing validation for bulk operations Committable suggestion
Suggested change
|
||||||
if (this.policyUtils.hasAuthGuard(this.model, 'postUpdate') || this.policyUtils.getZodSchema(this.model)) { | ||||||
// use a transaction to do post-update checks | ||||||
const postWriteChecks: PostWriteCheckRecord[] = []; | ||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -64,8 +64,8 @@ export default class DataModelValidator implements AstValidator<DataModel> { | |||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
const isArray = idField.type.array; | ||||||||||||||||||||||||||||||||||||
const isScalar = SCALAR_TYPES.includes(idField.type.type as typeof SCALAR_TYPES[number]) | ||||||||||||||||||||||||||||||||||||
const isValidType = isScalar || isEnum(idField.type.reference?.ref) | ||||||||||||||||||||||||||||||||||||
const isScalar = SCALAR_TYPES.includes(idField.type.type as (typeof SCALAR_TYPES)[number]); | ||||||||||||||||||||||||||||||||||||
const isValidType = isScalar || isEnum(idField.type.reference?.ref); | ||||||||||||||||||||||||||||||||||||
Comment on lines
+67
to
+68
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The logic for determining if a field's type is scalar or an enum is correctly implemented. However, consider adding a comment explaining why these specific checks are necessary for fields with + // Ensure @id fields are either scalar types or enums for database compatibility
const isScalar = SCALAR_TYPES.includes(idField.type.type as (typeof SCALAR_TYPES)[number]);
const isValidType = isScalar || isEnum(idField.type.reference?.ref); Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
if (isArray || !isValidType) { | ||||||||||||||||||||||||||||||||||||
accept('error', 'Field with @id attribute must be of scalar or enum type', { node: idField }); | ||||||||||||||||||||||||||||||||||||
|
@@ -121,7 +121,7 @@ export default class DataModelValidator implements AstValidator<DataModel> { | |||||||||||||||||||||||||||||||||||
fields = (arg.value as ArrayExpr).items as ReferenceExpr[]; | ||||||||||||||||||||||||||||||||||||
if (fields.length === 0) { | ||||||||||||||||||||||||||||||||||||
if (accept) { | ||||||||||||||||||||||||||||||||||||
accept('error', `"fields" value cannot be emtpy`, { | ||||||||||||||||||||||||||||||||||||
accept('error', `"fields" value cannot be empty`, { | ||||||||||||||||||||||||||||||||||||
node: arg, | ||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
@@ -131,7 +131,7 @@ export default class DataModelValidator implements AstValidator<DataModel> { | |||||||||||||||||||||||||||||||||||
references = (arg.value as ArrayExpr).items as ReferenceExpr[]; | ||||||||||||||||||||||||||||||||||||
if (references.length === 0) { | ||||||||||||||||||||||||||||||||||||
if (accept) { | ||||||||||||||||||||||||||||||||||||
accept('error', `"references" value cannot be emtpy`, { | ||||||||||||||||||||||||||||||||||||
accept('error', `"references" value cannot be empty`, { | ||||||||||||||||||||||||||||||||||||
node: arg, | ||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
@@ -157,6 +157,17 @@ export default class DataModelValidator implements AstValidator<DataModel> { | |||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||
for (let i = 0; i < fields.length; i++) { | ||||||||||||||||||||||||||||||||||||
if (!field.type.optional && fields[i].$resolvedType?.nullable) { | ||||||||||||||||||||||||||||||||||||
// if relation is not optional, then fk field must not be nullable | ||||||||||||||||||||||||||||||||||||
if (accept) { | ||||||||||||||||||||||||||||||||||||
accept( | ||||||||||||||||||||||||||||||||||||
'error', | ||||||||||||||||||||||||||||||||||||
`relation "${field.name}" is not optional, but field "${fields[i].target.$refText}" is optional`, | ||||||||||||||||||||||||||||||||||||
{ node: fields[i].target.ref! } | ||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||
Comment on lines
+160
to
+167
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This block of code introduces a validation check to ensure that if a relation is not optional, then the foreign key field must not be nullable. This is an important check for data integrity and consistency. However, consider adding a brief comment to explain the rationale behind this check for future maintainability. + // Validate non-optional relations to ensure foreign key fields are not nullable
if (!field.type.optional && fields[i].$resolvedType?.nullable) {
if (accept) {
accept(
'error',
`relation "${field.name}" is not optional, but field "${fields[i].target.$refText}" is optional`,
{ node: fields[i].target.ref! }
);
}
} Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
if (!fields[i].$resolvedType) { | ||||||||||||||||||||||||||||||||||||
if (accept) { | ||||||||||||||||||||||||||||||||||||
accept('error', `field reference is unresolved`, { node: fields[i] }); | ||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While iterating over
args.data
forcreateMany
operations, the validation and input check logic is sound. However, consider optimizing this loop to avoid potential performance issues with large datasets by minimizing the number of timesvalidateCreateInputSchema
andcheckInputGuard
are called.Committable suggestion