fix: several issues with using auth() in @default
#1088
Conversation
- Make generated TS field optional if it has a default - Handle the difference between save and unsafe Prisma mutation
WalkthroughWalkthroughThe recent updates focus on enhancing clarity, functionality, and security across various components of the system. Key improvements include clearer differentiation between foreign key fields and relation fields, advanced handling of default values with authentication attributes, and refined logic for identifying unsafe mutations. Additionally, new utilities and tests have been introduced to support these enhancements, ensuring a more robust and intuitive framework for managing data models, authentication, and policy enforcement. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 5
Configuration used: CodeRabbit UI
Files selected for processing (12)
- packages/runtime/src/cross/model-meta.ts (1 hunks)
- packages/runtime/src/enhancements/default-auth.ts (3 hunks)
- packages/runtime/src/enhancements/policy/handler.ts (3 hunks)
- packages/runtime/src/enhancements/utils.ts (2 hunks)
- packages/schema/src/plugins/enhancer/enhance/index.ts (4 hunks)
- packages/schema/src/plugins/enhancer/enhancer-utils.ts (1 hunks)
- packages/schema/src/plugins/prisma/schema-generator.ts (3 hunks)
- packages/schema/tests/schema/validation/attribute-validation.test.ts (3 hunks)
- packages/sdk/src/model-meta-generator.ts (3 hunks)
- packages/sdk/src/utils.ts (1 hunks)
- packages/testtools/src/schema.ts (2 hunks)
- tests/integration/tests/enhancements/with-policy/auth.test.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/schema/tests/schema/validation/attribute-validation.test.ts
Additional comments: 17
packages/schema/src/plugins/enhancer/enhancer-utils.ts (1)
- 5-20: The implementation of
isDefaultWithAuthfunction correctly identifies if a field attribute is a@defaultwithauth()invocation. This addition aligns with the PR's objectives to improve handling of default values, especially withauth()annotations.packages/runtime/src/enhancements/utils.ts (1)
- 28-43: The
isUnsafeMutatefunction correctly identifies unsafe mutations by checking if the mutation arguments include auto-increment ID fields or foreign keys. This logic is crucial for maintaining data integrity and security. The function's implementation aligns with the PR's objectives to differentiate between safe and unsafe Prisma mutations.The
isAutoIncrementIdFieldfunction accurately identifies if a field is an auto-increment ID field. This utility function is essential for theisUnsafeMutatefunction to determine unsafe mutations.Both functions are well-implemented and contribute to the PR's goals of enhancing mutation safety checks.
packages/runtime/src/cross/model-meta.ts (1)
- 78-85: The updated comments and descriptions for
relationFieldandforeignKeyMappingwithin theFieldInfotype provide clearer differentiation between foreign key fields and relation fields. These clarifications are beneficial for developers working with the codebase, enhancing understandability and maintainability.packages/runtime/src/enhancements/default-auth.ts (1)
- 91-137: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [72-124]
The enhancements in
default-auth.tsfor handling default values with@default(auth())are well-implemented. The use ofrequireFieldandisUnsafeMutatefunctions improves the robustness of setting default values, especially for foreign key fields. The logic to conditionally set default values based on whether a mutation is considered unsafe is a significant improvement, aligning with the PR's objectives to ensure data integrity and security.One area of improvement is to ensure that the handling of foreign key fields and the construction of the
{ connect: { ... } }payload are thoroughly tested, especially in edge cases where relation fields might have complex mappings.packages/testtools/src/schema.ts (1)
- 124-124: The addition of the
extraSourceFilesproperty toSchemaLoadOptionsis a useful enhancement that supports additional file handling during schema loading. This change enables more flexible testing scenarios by allowing the inclusion of extra source files directly through the schema loading options.packages/sdk/src/model-meta-generator.ts (2)
- 35-35: The import of
getRelationFieldis correctly added to support the new functionality related to relation fields handling. This aligns with the PR objectives of enhancing the codebase to support changes in handling default values and relation fields.- 251-255: The addition of logic to handle relation fields in the
writeFieldsfunction is a significant improvement. It ensures that relation fields are correctly identified and their related information is written, enhancing the framework's flexibility and developer experience. However, it's important to ensure that thegetRelationFieldfunction is thoroughly tested, especially in edge cases where a field might not have a direct relation field or in complex relation scenarios.packages/sdk/src/utils.ts (2)
- 284-304: The addition of the
getForeignKeyFieldsfunction is a valuable enhancement for extracting foreign key fields from relation fields. This function will facilitate more granular control and understanding of data model relationships, which is crucial for the framework's integrity and developer usability. Ensure comprehensive testing is conducted, particularly for data models with complex relationships and multiple foreign keys.- 306-321: The
getRelationFieldfunction is a crucial addition for identifying the relation field associated with a given foreign key field. This functionality enhances the framework's ability to handle data model relationships more effectively. Similar to thegetForeignKeyFieldsfunction, it's important to ensure this function is well-tested across various scenarios, including those with complex or nested relationships.packages/schema/src/plugins/enhancer/enhance/index.ts (3)
- 30-30: The import of
isDefaultWithAuthfrom../enhancer-utilsis a good addition, ensuring that the logic for checking default values withauth()is encapsulated and reusable.- 39-39: The logic to determine if a logical Prisma client is needed by calling
needsLogicalClientis clear and modular, improving the maintainability of the code.- 90-105: The implementation of
needsLogicalClient,hasDelegateModel, andhasAuthInDefaultfunctions is logically sound and aligns with the PR's objectives to enhance handling of default values and delegate models. These functions collectively improve the framework's flexibility and safety.tests/integration/tests/enhancements/with-policy/auth.test.ts (2)
- 538-572: The test case
Default auth() field optionalitycorrectly demonstrates the intended behavior of making fields optional if they have a default value specified usingauth(). However, it's important to ensure that theextraSourceFilesconfiguration and the simulated usage inmain.tsaccurately reflect real-world usage patterns and are correctly integrated into the test suite.
- Ensure that the
extraSourceFilesconfiguration is properly handled by the test suite setup and that the content ofmain.tsis executed as part of the test.- Verify that the Prisma client and the
enhancefunction are correctly imported and used withinmain.ts.- Confirm that the absence of
authorandauthorIdin thedb.post.createcall does not lead to any runtime errors or unexpected behavior, given that these fields are made optional by the defaultauth().id.
- 574-619: The test case
Default auth() safe unsafe mixaims to validate the differentiation between safe and unsafe Prisma mutations in the context of usingauth()within@defaultannotations. This test case is crucial for ensuring data integrity and security. However, there are a few points to consider:
- The test setup involves creating a
UserandStatsmodel instances before testing the creation of aPostmodel instance with a defaultauth().idand a relation toStats. It's important to ensure that the logic for determining whether a mutation is safe or unsafe is accurately represented in this test.- The distinction between "safe" and "unsafe" mutations in this context should be clearly documented within the test case or linked to the relevant documentation or code comments for clarity.
- Verify that the test accurately captures the intended behavior and edge cases for safe and unsafe mutations, especially in scenarios where the default value is derived from the
auth()function.packages/schema/src/plugins/prisma/schema-generator.ts (2)
- 498-513: The logic for making fields optional in logical mode when they have a
@defaultwithauth()or are foreign key fields related to such fields has been updated. This change aligns with the PR's objective to improve handling of default values withauth()annotations. However, it's crucial to ensure that this logic does not inadvertently make fields optional that should remain required for data integrity purposes.
- Ensure that making fields optional does not conflict with the data model's integrity requirements, especially in scenarios where a field should not be optional despite having a default value.
- Verify that this change is consistently applied across all relevant parts of the codebase to maintain uniform behavior.
- 518-518: The filter condition to exclude
@defaultattributes withauth()from being handled by Prisma directly is a critical part of ensuring that default values withauth()are processed correctly outside of Prisma. This change is necessary to support the PR's objectives and ensures that the framework can handle these default values in a more controlled manner.
- Confirm that all instances where
@defaultwithauth()should be handled outside Prisma are correctly identified and excluded by this filter.- Review related code to ensure that the logic for handling these default values outside of Prisma is robust and covers all necessary cases.
packages/runtime/src/enhancements/policy/handler.ts (1)
- 694-694: The refactoring of
isUnsafeMutateto a standalone function in../utilsand its usage here appears to be a positive change for modularity and reusability. Ensure that the implementation in../utilscorrectly handles all edge cases related to determining unsafe mutations and integrates well with the policy handling logic.
| import { execPackage } from '../../../utils/exec-utils'; | ||
| import { trackPrismaSchemaError } from '../../prisma'; | ||
| import { PrismaSchemaGenerator } from '../../prisma/schema-generator'; | ||
| import { isDefaultWithAuth } from '../enhancer-utils'; | ||
|
|
||
| // information of delegate models and their sub models | ||
| type DelegateInfo = [DataModel, DataModel[]][]; |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [48-48]
Detected multiple instances where user input might be indirectly influencing file paths through variables like outDir. Ensure that outDir and similar variables are sanitized or validated to prevent path traversal vulnerabilities.
- path.join(outDir, 'prisma.d.ts')
+ path.join(sanitizePath(outDir), 'prisma.d.ts')Note: Apply similar changes to all instances where outDir is used in path operations.
Also applies to: 56-56, 64-64, 110-110, 136-136, 148-148, 163-163
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [329-331]
The use of dynamically constructed regular expressions in removeCreateFromDelegateInput and removeDiscriminatorFromConcreteInput functions could lead to Regular Expression Denial-of-Service (ReDoS) vulnerabilities if the input is controlled by an attacker. Consider using hardcoded regexes or performing input validation to mitigate this risk.
- new RegExp(`\\${delegateModelNames.join('|')}(Unchecked)?(Create|Update).*Input`)
+ // Consider replacing with hardcoded regexes or validating `delegateModelNames` to ensure they do not form a vulnerable regex pattern.Also applies to: 352-354
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Gets the foreign key fields of the given relation field. | ||
| */ | ||
| export function getForeignKeyFields(relationField: DataModelField) { | ||
| if (!isRelationshipField(relationField)) { | ||
| return []; | ||
| } | ||
|
|
||
| const relAttr = relationField.attributes.find((attr) => attr.decl.ref?.name === '@relation'); | ||
| if (relAttr) { | ||
| // find "fields" arg | ||
| const fieldsArg = getAttributeArg(relAttr, 'fields'); | ||
| if (fieldsArg && isArrayExpr(fieldsArg)) { | ||
| return fieldsArg.items | ||
| .filter((item): item is ReferenceExpr => isReferenceExpr(item)) | ||
| .map((item) => item.target.ref as DataModelField); | ||
| } | ||
| } | ||
|
|
||
| return []; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the relation field of the given foreign key field. | ||
| */ | ||
| export function getRelationField(fkField: DataModelField) { | ||
| const model = fkField.$container as DataModel; | ||
| return model.fields.find((f) => { | ||
| const relAttr = f.attributes.find((attr) => attr.decl.ref?.name === '@relation'); | ||
| if (relAttr) { | ||
| const fieldsArg = getAttributeArg(relAttr, 'fields'); | ||
| if (fieldsArg && isArrayExpr(fieldsArg)) { | ||
| return fieldsArg.items.some((item) => isReferenceExpr(item) && item.target.ref === fkField); | ||
| } | ||
| } | ||
| return false; | ||
| }); | ||
| } | ||
|
|
||
| export function resolvePath(_path: string, options: Pick<PluginOptions, 'schemaPath'>) { | ||
| if (path.isAbsolute(_path)) { | ||
| return _path; |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [327-327]
The resolvePath function uses user input in a path.resolve call, which could potentially lead to a path traversal vulnerability. It's crucial to ensure that any user input is properly sanitized or validated before being used in file path operations to prevent unauthorized access to the file system. Consider implementing input validation or using a safer approach to handle file paths.
- return path.resolve(path.dirname(options.schemaPath), _path);
+ // Ensure _path is sanitized or validated before use
+ return path.resolve(path.dirname(options.schemaPath), sanitizePath(_path));| getPrismaOnly?: boolean; | ||
| enhancements?: EnhancementKind[]; | ||
| enhanceOptions?: Partial<EnhancementOptions>; | ||
| extraSourceFiles?: { name: string; content: string }[]; | ||
| }; | ||
|
|
||
| const defaultOptions: SchemaLoadOptions = { |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [40-40]
The use of execSync with the cmd parameter derived from function arguments raises a potential security concern related to command injection. Ensure that the cmd parameter is either not user-controllable or is properly sanitized to prevent malicious command execution.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [64-64]
The use of path.join with potentially user-controllable input in getWorkspaceRoot and getWorkspaceNpmCacheFolder functions could lead to path traversal vulnerabilities. Ensure that any user input is properly sanitized or validated to prevent unauthorized file system access.
Also applies to: 75-75
Summary by CodeRabbit
@default(auth())attribute handling, including better default value management for fields and foreign key fields.isUnsafeMutatefunction.PrismaSchemaGeneratorto refine the handling of default values withauth()annotations.auth()field optionality and scenarios involving a mix of safe and unsafe default values.enhancer-utils.tsfor specific utility functions likeisDefaultWithAuth.SchemaLoadOptionsto support additional source files during schema loading.