feat: centralize validation schemas #2669
Conversation
chronark
requested review from
perkinsjr,
mcstepp and
MichaelUnkey
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
Caution Review failedThe pull request is closed. 📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (4)
internal/validation/src/index.ts (1)
1-46: Consider standardizing error handling across applications.Since this validation module is used across multiple applications (api and dashboard), consider:
- Adding custom error messages that are user-friendly and consistent
- Creating a shared error handling utility to standardize how validation errors are processed and displayed
- Adding examples in the documentation showing how to properly handle validation errors in both API and UI contexts
Would you like me to provide an example implementation of standardized error handling?
🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 37-37: Overly permissive regular expression range
Suspicious character range that overlaps with A-Z in the same character class, and is equivalent to [0-9:;<=>?@a-z].apps/api/src/routes/v1_permissions_createRole.ts (1)
Line range hint
67-67: Review the role ID prefix "test"The role ID is being generated with a "test" prefix:
newId("test"). This seems inappropriate for a production route and could lead to confusion or issues with role identification.Consider using a more appropriate prefix like "role_" instead:
- id: newId("test"), + id: newId("role"),apps/dashboard/app/(app)/authorization/roles/[roleId]/update-role.tsx (1)
42-43: Good architectural decision: Centralized validation.Moving validation to a shared package (
@unkey/validation) brings several benefits:
- Ensures consistent validation between frontend and backend
- Creates a single source of truth for validation rules
- Simplifies maintenance and updates of validation logic
apps/dashboard/app/(app)/authorization/roles/create-new-role.tsx (1)
Line range hint
147-186: Consider documenting the status of permission management.There's a commented-out section for permission management that's marked as "Broken". This might affect the completeness of the role creation workflow.
Consider adding a TODO comment with more context about:
- The current status of this feature
- Any tracking issues or future plans
- Whether roles can be properly used without this functionality
- {/* Broken Have to link permissions on next page after creation */} + {/* TODO: Permission selection is currently disabled - permissions must be linked on the next page after creation + Tracking issue: <issue_number> + Impact: Roles can still be created and permissions can be added later + */}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
apps/api/package.json(1 hunks)apps/api/src/routes/v1_permissions_createPermission.ts(2 hunks)apps/api/src/routes/v1_permissions_createRole.ts(2 hunks)apps/dashboard/app/(app)/authorization/permissions/create-new-permission.tsx(1 hunks)apps/dashboard/app/(app)/authorization/roles/[roleId]/update-role.tsx(2 hunks)apps/dashboard/app/(app)/authorization/roles/create-new-role.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/update-namespace-name.tsx(1 hunks)apps/dashboard/package.json(1 hunks)internal/validation/README.md(1 hunks)internal/validation/package.json(1 hunks)internal/validation/src/index.ts(1 hunks)internal/validation/tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- internal/validation/package.json
- internal/validation/tsconfig.json
🧰 Additional context used
🪛 LanguageTool
internal/validation/README.md
[typographical] ~3-~3: After the expression ‘for example’ a comma is usually used.
Context: ...shared schema or validation utils. For example user-chosen identifiers.
(COMMA_FOR_EXAMPLE)
🪛 GitHub Check: CodeQL
internal/validation/src/index.ts
[warning] 37-37: Overly permissive regular expression range
Suspicious character range that overlaps with A-Z in the same character class, and is equivalent to [0-9:;<=>?@a-z].
🔇 Additional comments (17)
internal/validation/src/index.ts (3)
3-14: Well-structured identifier schema with clear constraints!
The schema effectively enforces URL-safe characters and reasonable length limits, with clear documentation explaining its purpose.
25-32: Apply the same character restrictions to description schema.
Similar to the name schema, consider adding character restrictions for better security.
41-46: LGTM! Clean and well-organized exports.
The validation object provides a clear interface for accessing all schemas.
apps/api/package.json (1)
40-40: LGTM! Workspace dependency correctly configured.
The addition of @unkey/validation as a workspace dependency is properly configured and aligns with the PR's objective of centralizing validation schemas.
Let's verify the workspace setup and package existence:
✅ Verification successful
Workspace dependency setup is correctly configured ✅
The verification confirms:
- The
@unkey/validationpackage exists in the workspace - The package is properly used as a dependency by:
@unkey/dashboardapi(the current package)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the workspace package setup for @unkey/validation
# Check if the validation package exists in the workspace
fd -t f "package.json" -x jq -r 'select(.name=="@unkey/validation") | .name' {}
# Verify that other packages using @unkey/validation have it in their dependencies
fd -t f "package.json" -x jq -r 'select(.dependencies."@unkey/validation" != null) | .name' {}
Length of output: 217
apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/update-namespace-name.tsx (2)
19-19: LGTM! Import of centralized validation module
The addition of @unkey/validation import aligns with the PR's objective of centralizing validation schemas.
24-26: Verify validation rules compatibility
The migration to centralized validation schemas looks good, but please verify:
- The validation rules in
validation.namematch the previous requirements - The
validation.unkeyIdformat is compatible with existing namespace and workspace IDs
✅ Verification successful
Validation rules are compatible and more restrictive
The migration to centralized validation rules is safe and actually more restrictive than the previous implementation:
Previous validation:
- name: 3-50 chars, alphanumeric with
_-. - namespaceId/workspaceId: just string type
New validation:
- name: 3-256 chars (more permissive length but still covers min requirement)
- unkeyId: enforces strict format
/^[a-z]{3,4}_[a-zA-Z0-Z]{8,}$/(more restrictive)
The validation rules are used consistently across the codebase for roles, permissions, and namespaces, ensuring uniform validation behavior.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify validation rules and their usage
# Check the validation rules implementation
echo "Checking validation rules implementation:"
ast-grep --pattern 'export const validation = {
name: $_,
unkeyId: $_
}'
# Check for existing ID formats in the codebase
echo "Checking existing ID formats:"
rg -A 1 'namespace\.id|workspaceId' --type typescript
Length of output: 332
Script:
#!/bin/bash
# Let's try a different approach to find validation rules
# Search for validation schema definitions
echo "Searching for validation schema definitions:"
rg -l "validation.*name.*unkeyId"
# Look for specific validation rules
echo "Looking for name and ID validation patterns:"
rg "z\.string\(\)|z\.object\(\)" -A 3
# Check for namespace and workspace ID usage patterns
echo "Checking ID usage patterns:"
rg "(namespace|workspace)Id.*=.*" -A 1
# Find any validation-related imports
echo "Checking validation-related imports:"
rg "import.*validation" -A 1
Length of output: 189163
Script:
#!/bin/bash
# Let's check the validation rules in the @unkey/validation package
# Find the validation rules implementation
echo "Checking validation rules in @unkey/validation:"
rg -A 5 "export const validation" internal/validation/src/
# Check the previous validation rules in the file
echo "Checking previous validation rules in the file:"
git log -p apps/dashboard/app/\(app\)/ratelimits/\[namespaceId\]/settings/update-namespace-name.tsx
Length of output: 5950
Script:
#!/bin/bash
# Let's check the specific validation rules in the validation package
echo "Checking name and unkeyId validation rules:"
rg -A 10 "const name =|const unkeyId =" internal/validation/src/
# Check if these validation rules are used consistently across the codebase
echo "Checking usage of validation rules:"
rg "validation\.(name|unkeyId)" -B 2 -A 2
Length of output: 4084
apps/api/src/routes/v1_permissions_createRole.ts (1)
10-10: LGTM: Import of centralized validation package
The addition of @unkey/validation import aligns with the PR objective of centralizing validation schemas.
apps/api/src/routes/v1_permissions_createPermission.ts (4)
4-4: LGTM: Import aligns with centralization objective
The addition of the centralized validation import aligns with the PR's objective of standardizing validation schemas.
Line range hint 4-34: LGTM: Well-structured implementation with proper safeguards
The changes maintain the robust implementation including:
- Proper error handling through openApiErrorResponses
- Transaction management for data consistency
- Comprehensive audit logging
Line range hint 29-34: Verify optional description behavior
The description field is now explicitly marked as optional. Please verify if this is an intentional change in behavior and ensure it's consistently applied across all permission-related endpoints.
✅ Verification successful
The search results show that the description field is consistently marked as optional across all permission and role-related endpoints in both the API and dashboard components. Let me verify if there are any other validation patterns we might have missed.
Optional description field is consistently implemented across the codebase
The verification shows that the description field is consistently marked as optional across all permission and role-related endpoints:
- API routes for creating permissions and roles
- Dashboard components for creating and updating permissions and roles
- Client-side validation schemas
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistency in description field validation
# Look for other permission endpoints
rg -A 5 'description.*validation\.description\.optional\(\)'
# Check for any required description fields that might conflict
rg -A 5 'description.*validation\.description[^.]'
Length of output: 3221
Script:
#!/bin/bash
# Search for any other validation patterns for description fields in permissions/roles
rg -A 5 'description:.*validation\.'
# Search for any schema definitions related to permissions/roles
ast-grep --pattern 'const $_ = z.object({
$$$
description: $_,
$$$
})'
Length of output: 8056
25-28: Verify validation rules for identifier
The change to use validation.identifier needs verification to ensure it maintains or improves upon the previous validation rules for permission names.
apps/dashboard/app/(app)/authorization/permissions/create-new-permission.tsx (3)
40-41: LGTM! Clean refactoring to centralized validation
The changes effectively move validation logic to a centralized location while maintaining type safety and component behavior. The refactoring is clean and focused.
Also applies to: 30-30
40-41: Verify validation rules haven't been weakened
The schema is now more maintainable, but let's verify the validation rules in the centralized schema.
#!/bin/bash
# Description: Verify validation rules in @unkey/validation package
# Find and display the validation schema definition
ast-grep --pattern 'export const validation = {
name: $$$,
description: $$$
}'
# Alternatively, search for individual validation rules
rg -A 5 'name:|description:' 30-30: Verify @unkey/validation package setup
Let's ensure the package is properly configured in the monorepo workspace.
apps/dashboard/app/(app)/authorization/roles/[roleId]/update-role.tsx (1)
30-30: LGTM: Clean import of centralized validation.
The import statement aligns with the PR's objective of centralizing validation schemas.
apps/dashboard/app/(app)/authorization/roles/create-new-role.tsx (2)
31-31: LGTM! Import of centralized validation module.
The addition of the centralized validation import aligns well with the PR's objective of standardizing validation schemas across the application.
42-44: LGTM! Validation rules properly centralized.
The validation rules for name and description fields have been successfully migrated to use the centralized validation schema.
Let's verify the consistency of these validation rules across the application:
✅ Verification successful
Let me analyze the validation rules usage across the codebase to verify consistency.
Validation rules are properly centralized and consistently used
The verification confirms that:
- The centralized validation rules in
internal/validation/src/index.tsdefine consistent constraints:name: string with min(3) and max(256)description: string with min(3) and max(256)
- All RBAC-related components (roles and permissions) consistently use these centralized rules through
validation.nameandvalidation.description.optional() - The optional modifier for description is correctly and consistently applied across the codebase
The changes align with the existing validation patterns in the codebase and maintain consistency in validation rules.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of validation rules across the codebase
# Search for any remaining inline validation for 'name' fields
echo "Checking for potential inconsistent name validations:"
rg "name.*z\.(string|object)" -g "*.{ts,tsx}"
# Search for any remaining inline validation for 'description' fields
echo "Checking for potential inconsistent description validations:"
rg "description.*z\.(string|object)" -g "*.{ts,tsx}"
# Check the actual validation rules being used
echo "Checking current validation usage:"
rg "validation\.(name|description)" -g "*.{ts,tsx}"
Length of output: 12595
Script:
#!/bin/bash
# Let's check the centralized validation rules definition
echo "Checking validation rules definition:"
rg -A 5 "const (name|description) = z\.string" internal/validation/src/index.ts
# Check if there are any other validation rule exports
echo -e "\nChecking validation exports:"
rg "export.*validation" internal/validation/src/index.ts
# Check specific usage in RBAC-related files
echo -e "\nChecking RBAC-specific validation usage:"
rg "name:|description:" apps/dashboard/app/\(app\)/authorization/ -A 1
Length of output: 6100
internal/validation/src/index.ts
Outdated
| const unkeyId = z | ||
| .string() | ||
| .regex( | ||
| /^[a-z]{3,4}_[a-zA-Z0-Z]{8,}$/, | ||
| "Unkey IDs must include a prefix, separated by an underscore: key_abcdefg123", | ||
| ); |
There was a problem hiding this comment.
Fix regex pattern in unkeyId schema.
The current regex pattern has two issues:
- Typo in character class:
[a-zA-Z0-Z]should be[a-zA-Z0-9] - The length constraint for the second part is unbounded, which could allow very long IDs
- /^[a-z]{3,4}_[a-zA-Z0-Z]{8,}$/,
+ /^[a-z]{3,4}_[a-zA-Z0-9]{8,16}$/,This fix:
- Corrects the typo in the character class
- Adds an upper bound of 16 characters for the second part to prevent excessively long IDs
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const unkeyId = z | |
| .string() | |
| .regex( | |
| /^[a-z]{3,4}_[a-zA-Z0-Z]{8,}$/, | |
| "Unkey IDs must include a prefix, separated by an underscore: key_abcdefg123", | |
| ); | |
| const unkeyId = z | |
| .string() | |
| .regex( | |
| /^[a-z]{3,4}_[a-zA-Z0-9]{8,16}$/, | |
| "Unkey IDs must include a prefix, separated by an underscore: key_abcdefg123", | |
| ); |
🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 37-37: Overly permissive regular expression range
Suspicious character range that overlaps with A-Z in the same character class, and is equivalent to [0-9:;<=>?@a-z].
| /** | ||
| * A name is a user given human-readable string. | ||
| * | ||
| * It must not be used in URLs. | ||
| * | ||
| * @example the name of a key | ||
| */ | ||
| const name = z.string().min(3).max(256); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding character restrictions for the name schema.
While the documentation states it "must not be used in URLs", there are no character restrictions preventing unsafe characters. Consider adding a regex pattern to enforce safe characters, similar to the identifier schema.
-const name = z.string().min(3).max(256);
+const name = z
+ .string()
+ .min(3)
+ .max(256)
+ .regex(
+ /^[^<>{}[\]\\\/]*$/,
+ "Name cannot contain special characters like <, >, {, }, [, ], \\, or /"
+ );📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * A name is a user given human-readable string. | |
| * | |
| * It must not be used in URLs. | |
| * | |
| * @example the name of a key | |
| */ | |
| const name = z.string().min(3).max(256); | |
| /** | |
| * A name is a user given human-readable string. | |
| * | |
| * It must not be used in URLs. | |
| * | |
| * @example the name of a key | |
| */ | |
| const name = z | |
| .string() | |
| .min(3) | |
| .max(256) | |
| .regex( | |
| /^[^<>{}[\]\\\/]*$/, | |
| "Name cannot contain special characters like <, >, {, }, [, ], \\, or /" | |
| ); |
| name: validation.name.openapi({ | ||
| description: "The unique name of your role.", | ||
| example: "dns.records.manager", | ||
| }), | ||
| description: validation.description.optional().openapi({ |
There was a problem hiding this comment.
💡 Codebase verification
Validation rules have been relaxed and need adjustment
The previous implementation had stricter validation rules for the role name:
- Old: Required regex pattern
/^[a-zA-Z0-9_:\-\.\*]+$/with specific error message - New: Only enforces min length (3) and max length (256) without pattern validation
The centralized validation should be updated to maintain the same level of strictness for role names to prevent potential security or consistency issues.
internal/validation/src/index.ts: Thenamevalidator should include the regex pattern for role names
🔗 Analysis chain
Verify validation rules consistency
The schema now uses centralized validation methods. Let's ensure the validation rules remain consistent with the previous implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Compare the validation rules between old and new implementation
# Check for previous validation rules in git history
git log -p --full-history -S "name:" -- "apps/api/src/routes/v1_permissions_createRole.ts" | grep -A 10 "name:"
# Check the validation implementation in the new package
fd -e ts -e js "validation" -x cat {} \; | grep -A 10 "name ="
Length of output: 1419
Script:
#!/bin/bash
# Let's try a different approach to find the validation rules
# Search for validation rules in the current implementation
ast-grep --pattern 'const validation = {
$$$
}'
# Also search for any validation-related imports
rg "import.*validation" -A 2
Length of output: 2289
Script:
#!/bin/bash
# Let's examine the validation rules in the centralized validation package
# Check the validation rules for name and description
cat internal/validation/src/index.ts
# Also check if there are any other validation-related files
fd -e ts -e js validation --full-path --exec cat {} \;
Length of output: 2012
| @@ -53,6 +53,7 @@ | |||
| "@unkey/rbac": "workspace:^", | |||
| "@unkey/resend": "workspace:^", | |||
| "@unkey/schema": "workspace:^", | |||
| "@unkey/validation": "workspace:^", | |||
There was a problem hiding this comment.
💡 Codebase verification
Warning: @unkey/validation package is missing but being imported
The package is being imported in multiple files but appears to be missing from the workspace:
- No package found at
packages/validation - Multiple files are already importing from "@unkey/validation":
- apps/dashboard/app/(app)/authorization/permissions/create-new-permission.tsx
- apps/dashboard/app/(app)/authorization/roles/create-new-role.tsx
- apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/update-namespace-name.tsx
- apps/dashboard/app/(app)/authorization/roles/[roleId]/update-role.tsx
- apps/api/src/routes/v1_permissions_createRole.ts
- apps/api/src/routes/v1_permissions_createPermission.ts
The workspace dependency cannot be added until the package is created and properly set up in the monorepo.
🔗 Analysis chain
LGTM! Verify the @unkey/validation package setup.
The addition of @unkey/validation as a workspace dependency is correctly positioned and properly versioned.
Let's verify the package setup:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the @unkey/validation package existence and configuration
# Check if the package.json exists for @unkey/validation
fd --type f "package.json" | grep -l "validation" | xargs cat
# Check for any existing imports of validation schemas
rg "@unkey/validation" --type ts --type tsx
Length of output: 216
Script:
#!/bin/bash
# Let's try a different approach to verify the package setup
# Check if validation package exists in packages directory
fd "package.json" packages/
# Check for any TypeScript files importing from @unkey/validation
rg -t ts "@unkey/validation"
# Look for validation-related files in packages directory
fd "validation" packages/
Length of output: 968
Script:
#!/bin/bash
# Let's check the root packages directory specifically for validation package
ls -la packages/validation 2>/dev/null || echo "Validation package not found"
# Check the workspace configuration in root package.json
cat package.json | grep -A 5 "workspaces"
# Check if there are any validation schemas in a potential validation package
fd . packages/validation --type f 2>/dev/null || echo "No validation package files found"
Length of output: 391
| name: validation.name, | ||
| description: validation.description.optional(), |
There was a problem hiding this comment.
💡 Codebase verification
Now I can make a determination. The validation rules are equivalent:
Previous validation:
name: z.string().min(3).regex(/^[a-zA-Z0-9_:\-\.\*]+$/, {
message: "Must be at least 3 characters long and only contain alphanumeric, colons, periods, dashes and underscores"
}),
description: z.string().optional()New centralized validation from @unkey/validation:
name: z.string().min(3).max(256),
description: z.string().min(3).max(256)The new validation rules are actually more strict for the description field (requiring min 3 chars instead of being completely optional) and add a max length constraint of 256 chars for both fields. The name field drops the regex pattern restriction but maintains the min length requirement.
Validation rules have changed and become more restrictive
The new schema:
- Adds max length validation (256 chars) for both fields
- Makes description field required with min length
- Removes special character restrictions on name field
🔗 Analysis chain
Verify validation rules equivalence.
The schema now uses centralized validation rules. Let's verify that these rules maintain the same validation constraints as before.
The previous attempt didn't show us the new validation rules. Let's try to locate the validation module and its contents to compare with the previous implementation we found, which required:
- name: min 3 chars, alphanumeric with specific special chars
- description: optional string
The previous attempt showed us the validation module is imported from multiple places. Let's find its implementation to compare with the previous validation rules we found in the git history.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Compare old and new validation rules
# Search for previous validation schema definition
echo "Previous validation rules:"
git log -p -S "formSchema = z.object" --full-history -- "apps/dashboard/app/(app)/authorization/roles/[roleId]/update-role.tsx"
# Check the new validation rules implementation
echo -e "\nNew validation rules from @unkey/validation:"
rg -A 10 "export const name = " --type ts
rg -A 10 "export const description = " --type ts
Length of output: 5722
Script:
#!/bin/bash
# Let's try a different approach to find the validation rules
# Search for validation rules in the entire codebase
echo "Searching for validation rules:"
fd -e ts -e tsx --exec grep -l "validation" {} \; | xargs cat
# Alternatively, let's try to find the specific import path
echo -e "\nSearching for validation import:"
rg "@unkey/validation" --type ts
Length of output: 259433
Script:
#!/bin/bash
# Let's check the validation module's implementation
echo "Searching for validation implementation:"
fd -e ts -e tsx --exec grep -l "export const validation" {} \; | xargs cat
# Also let's check the previous implementation of the form schema
echo -e "\nSearching for previous form schema in the file:"
git log -p -S "formSchema = z.object" -- "apps/dashboard/app/(app)/authorization/roles/[roleId]/update-role.tsx"
Length of output: 6683
…range Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
internal/validation/src/index.ts (1)
3-14: Enhance the identifier regex pattern for better URL safety.While the current pattern ensures basic URL safety, consider these improvements:
- Prevent consecutive special characters (like "..", "--", "__")
- Ensure it doesn't start or end with special characters
- /^[a-zA-Z0-9_\.:\-]*$/, + /^[a-zA-Z0-9][a-zA-Z0-9_\.:]+[a-zA-Z0-9]$/,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
internal/validation/src/index.ts(1 hunks)
🔇 Additional comments (4)
internal/validation/src/index.ts (4)
41-46: LGTM! Clean and well-organized exports.
The validation object provides a clear and organized API for consuming these schemas.
16-23: 🛠️ Refactor suggestion
Add character restrictions for the name schema.
The schema allows any characters despite the documentation stating it "must not be used in URLs". Consider adding character restrictions to prevent unsafe characters.
34-39:
Add upper bound to unkeyId pattern length.
The unbounded length in the second part of the pattern ({8,}) could allow very long IDs, potentially leading to DoS vulnerabilities.
25-32: 🛠️ Refactor suggestion
Align description schema restrictions with name schema.
For consistency, the description schema should have the same character restrictions as the name schema since both are human-readable strings that must not be used in URLs.
-const description = z.string().min(3).max(256);
+const description = z
+ .string()
+ .min(3)
+ .max(256)
+ .regex(
+ /^[^<>{}[\]\\\/]*$/,
+ "Description cannot contain special characters like <, >, {, }, [, ], \\, or /"
+ );Likely invalid or redundant comment.
Summary by CodeRabbit
Release Notes
New Features
@unkey/validationdependency for enhanced validation capabilities.Improvements
createPermission,createRole, and other components to use the new validation module, ensuring consistency and maintainability.Documentation
@unkey/validationpackage to describe its purpose and functionalities.