Skip to content

fix: validate zod schema before update operation is executed #1051

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 24, 2024
Merged

fix: validate zod schema before update operation is executed #1051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 56 additions & 9 deletions packages/runtime/src/enhancements/policy/handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,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.utils.getZodSchema(model, 'create');
if (schema) {
if (schema && data) {
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validateCreateInputSchema method correctly applies Zod schema validation for create operations. However, consider adding error handling for cases where Zod schema is not defined but expected, to prevent silent failures.

if (schema && data) {
+   // Existing logic
} else {
+   throw new Error(`Zod schema for model '${model}' and operation 'create' not found.`);
}
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if (schema && data) {
if (schema && data) {
// Existing logic
} else {
throw new Error(`Zod schema for model '${model}' and operation 'create' not found.`);
}

const parseResult = schema.safeParse(data);
if (!parseResult.success) {
throw this.utils.deniedByPolicy(
Expand Down Expand Up @@ -496,26 +496,29 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr

args = this.utils.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.utils.replace(item, validationResult);
}

const inputCheck = this.utils.checkInputGuard(this.model, item, 'create');
if (inputCheck === false) {
// unconditionally deny
throw this.utils.deniedByPolicy(
this.model,
'create',
undefined,
CrudFailureReason.ACCESS_POLICY_VIOLATION
);
} else if (inputCheck === true) {
const r = this.validateCreateInputSchema(this.model, item);
if (r !== item) {
this.utils.replace(item, r);
}
// unconditionally allow
Comment on lines +499 to +518
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loop for validating and checking input guards in createMany method is well-implemented. However, consider extracting this logic into a separate method to improve readability and maintainability, especially since similar patterns are used in other parts of the class.

+ private validateAndCheckInputGuards(model: string, data: any[]) {
+     let needPostCreateCheck = false;
+     for (const item of data) {
+         // Existing validation and check logic here
+     }
+     return needPostCreateCheck;
+ }
- // Original loop logic here
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
// 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.utils.replace(item, validationResult);
}
const inputCheck = this.utils.checkInputGuard(this.model, item, 'create');
if (inputCheck === false) {
// unconditionally deny
throw this.utils.deniedByPolicy(
this.model,
'create',
undefined,
CrudFailureReason.ACCESS_POLICY_VIOLATION
);
} else if (inputCheck === true) {
const r = this.validateCreateInputSchema(this.model, item);
if (r !== item) {
this.utils.replace(item, r);
}
// unconditionally allow
private validateAndCheckInputGuards(model: string, data: any[]) {
let needPostCreateCheck = false;
for (const item of data) {
// Existing validation and check logic here
}
return needPostCreateCheck;
}

} else if (inputCheck === undefined) {
// static policy check is not possible, need to do post-create check
needPostCreateCheck = true;
break;
}
}

Expand Down Expand Up @@ -786,7 +789,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.utils.replace(updatePayload, validatedPayload);
}

if (updatePayload) {
for (const key of Object.keys(updatePayload)) {
const field = resolveField(this.modelMeta, model, key);
Expand Down Expand Up @@ -857,6 +866,8 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
);
}

args.data = this.validateUpdateInputSchema(model, args.data);

const updateGuard = this.utils.getAuthGuard(db, model, 'update');
if (this.utils.isTrue(updateGuard) || this.utils.isFalse(updateGuard)) {
// injects simple auth guard into where clause
Expand Down Expand Up @@ -917,7 +928,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
Expand Down Expand Up @@ -1016,6 +1030,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.utils.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.utils.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;
Expand Down Expand Up @@ -1046,6 +1091,8 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
args = this.utils.clone(args);
this.utils.injectAuthGuardAsWhere(this.prisma, args, this.model, 'update');

args.data = this.validateUpdateInputSchema(this.model, args.data);

Comment on lines +1094 to +1095
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of validateUpdateInputSchema in updateMany method is appropriate. However, consider adding a comment explaining the rationale behind validating only literal fields, as this might not be immediately clear to other developers or future maintainers.

+ // Validate only literal fields to ensure compatibility with Prisma's special update operations
args.data = this.validateUpdateInputSchema(this.model, args.data);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
args.data = this.validateUpdateInputSchema(this.model, args.data);
// Validate only literal fields to ensure compatibility with Prisma's special update operations
args.data = this.validateUpdateInputSchema(this.model, args.data);

if (this.utils.hasAuthGuard(this.model, 'postUpdate') || this.utils.getZodSchema(this.model)) {
// use a transaction to do post-update checks
const postWriteChecks: PostWriteCheckRecord[] = [];
Expand Down
136 changes: 95 additions & 41 deletions tests/integration/tests/enhancements/with-policy/field-validation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe('With Policy: field validation', () => {
id String @id @default(cuid())
user User @relation(fields: [userId], references: [id])
userId String
slug String @regex("^[0-9a-zA-Z]{4,16}$")
slug String @regex("^[0-9a-zA-Z]{4,16}$") @lower

@@allow('all', true)
}
Expand Down Expand Up @@ -508,50 +508,104 @@ describe('With Policy: field validation', () => {
},
});

await expect(
db.userData.create({
data: {
userId: '1',
a: 1,
b: 0,
c: -1,
d: 0,
text1: 'abc123',
text2: 'def',
text3: 'aaa',
text4: 'abcab',
text6: ' AbC ',
text7: 'abc',
let ud = await db.userData.create({
data: {
userId: '1',
a: 1,
b: 0,
c: -1,
d: 0,
text1: 'abc123',
text2: 'def',
text3: 'aaa',
text4: 'abcab',
text6: ' AbC ',
text7: 'abc',
},
});
expect(ud).toMatchObject({ text6: 'abc', text7: 'ABC' });

ud = await db.userData.update({
where: { id: ud.id },
data: {
text4: 'xyz',
text6: ' bCD ',
text7: 'bcd',
},
});
expect(ud).toMatchObject({ text4: 'xyz', text6: 'bcd', text7: 'BCD' });

let u = await db.user.create({
data: {
id: '2',
password: 'abc123!@#',
email: 'who@myorg.com',
handle: 'user2',
userData: {
create: {
a: 1,
b: 0,
c: -1,
d: 0,
text1: 'abc123',
text2: 'def',
text3: 'aaa',
text4: 'abcab',
text6: ' AbC ',
text7: 'abc',
},
},
})
).resolves.toMatchObject({ text6: 'abc', text7: 'ABC' });
},
include: { userData: true },
});
expect(u.userData).toMatchObject({
text6: 'abc',
text7: 'ABC',
});

await expect(
db.user.create({
data: {
id: '2',
password: 'abc123!@#',
email: 'who@myorg.com',
handle: 'user2',
userData: {
create: {
a: 1,
b: 0,
c: -1,
d: 0,
text1: 'abc123',
text2: 'def',
text3: 'aaa',
text4: 'abcab',
text6: ' AbC ',
text7: 'abc',
},
u = await db.user.update({
where: { id: u.id },
data: {
userData: {
update: {
data: { text4: 'xyz', text6: ' bCD ', text7: 'bcd' },
},
},
include: { userData: true },
})
).resolves.toMatchObject({
userData: expect.objectContaining({ text6: 'abc', text7: 'ABC' }),
},
include: { userData: true },
});
expect(u.userData).toMatchObject({ text4: 'xyz', text6: 'bcd', text7: 'BCD' });

// upsert create
u = await db.user.update({
where: { id: u.id },
data: {
tasks: {
upsert: {
where: { id: 'unknown' },
create: { slug: 'SLUG1' },
update: {},
},
},
},
include: { tasks: true },
});
expect(u.tasks[0]).toMatchObject({ slug: 'slug1' });

// upsert update
u = await db.user.update({
where: { id: u.id },
data: {
tasks: {
upsert: {
where: { id: u.tasks[0].id },
create: {},
update: { slug: 'SLUG2' },
},
},
},
include: { tasks: true },
});
expect(u.tasks[0]).toMatchObject({ slug: 'slug2' });
});
});
Loading