Skip to content

feat: centralize validation schemas #2669

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
chronark merged 4 commits into from
Dec 2, 2024
Merged

feat: centralize validation schemas #2669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8f46c1d
feat: centralize validation schemas
chronark Nov 18, 2024
fefc63e
Fix code scanning alert no. 12: Overly permissive regular expression …
chronark Dec 2, 2024
d62e3dc
chore: sync lock file
chronark Dec 2, 2024
bbc2f7d
Merge branch 'validation' of https://github.com/unkeyed/unkey into va…
chronark Dec 2, 2024
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
1 change: 1 addition & 0 deletions apps/api/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"@unkey/metrics": "workspace:^",
"@unkey/rbac": "workspace:^",
"@unkey/schema": "workspace:^",
"@unkey/validation": "workspace:^",
"@unkey/worker-logging": "workspace:^",
"hono": "^4.6.3",
"superjson": "^2.2.1",
Expand Down
19 changes: 7 additions & 12 deletions apps/api/src/routes/v1_permissions_createPermission.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import type { App } from "@/pkg/hono/app";
import { createRoute, z } from "@hono/zod-openapi";

import { validation } from "@unkey/validation";

import { insertUnkeyAuditLog } from "@/pkg/audit";
import { rootKeyAuth } from "@/pkg/auth/root_key";
import { openApiErrorResponses } from "@/pkg/errors";
Expand All @@ -20,18 +22,11 @@ const route = createRoute({
content: {
"application/json": {
schema: z.object({
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",
})
.openapi({
description: "The unique name of your permission.",
example: "record.write",
}),
description: z.string().optional().openapi({
name: validation.identifier.openapi({
description: "The unique name of your permission.",
example: "record.write",
}),
description: validation.description.optional().openapi({
description:
"Explain what this permission does. This is just for your team, your users will not see this.",
example: "record.write can create new dns records for our domains.",
Expand Down
18 changes: 6 additions & 12 deletions apps/api/src/routes/v1_permissions_createRole.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { openApiErrorResponses } from "@/pkg/errors";
import { schema } from "@unkey/db";
import { newId } from "@unkey/id";
import { buildUnkeyQuery } from "@unkey/rbac";
import { validation } from "@unkey/validation";

const route = createRoute({
tags: ["permissions"],
Expand All @@ -20,18 +21,11 @@ const route = createRoute({
content: {
"application/json": {
schema: z.object({
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",
})
.openapi({
description: "The unique name of your role.",
example: "dns.records.manager",
}),
description: z.string().optional().openapi({
name: validation.name.openapi({
description: "The unique name of your role.",
example: "dns.records.manager",
}),
description: validation.description.optional().openapi({
Comment on lines +24 to +28
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 18, 2024

Choose a reason for hiding this comment

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

💡 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: The name validator 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

description:
"Explain what this role does. This is just for your team, your users will not see this.",
example: "dns.records.manager can read and write dns records for our domains.",
Expand Down
13 changes: 3 additions & 10 deletions apps/dashboard/app/(app)/authorization/permissions/create-new-permission.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,18 @@ import { toast } from "@/components/ui/toaster";
import { trpc } from "@/lib/trpc/client";
import { zodResolver } from "@hookform/resolvers/zod";
import { DialogTrigger } from "@radix-ui/react-dialog";
import { validation } from "@unkey/validation";
import { useRouter } from "next/navigation";
import { useState } from "react";
import { useForm } from "react-hook-form";
import { z } from "zod";

type Props = {
trigger: React.ReactNode;
};

const formSchema = z.object({
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(),
name: validation.name,
description: validation.description.optional(),
});

export const CreateNewPermission: React.FC<Props> = ({ trigger }) => {
Expand Down
11 changes: 3 additions & 8 deletions apps/dashboard/app/(app)/authorization/roles/[roleId]/update-role.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { trpc } from "@/lib/trpc/client";
import { zodResolver } from "@hookform/resolvers/zod";
import { DialogTrigger } from "@radix-ui/react-dialog";
import type { Role } from "@unkey/db";
import { validation } from "@unkey/validation";
import { useRouter } from "next/navigation";
import { useState } from "react";
import { useForm } from "react-hook-form";
Expand All @@ -38,14 +39,8 @@ type Props = {
};

const formSchema = z.object({
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(),
name: validation.name,
description: validation.description.optional(),
Comment on lines +42 to +43
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 18, 2024

Choose a reason for hiding this comment

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

💡 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:

  1. Adds max length validation (256 chars) for both fields
  2. Makes description field required with min length
  3. 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

});

export const UpdateRole: React.FC<Props> = ({ trigger, role }) => {
Expand Down
12 changes: 3 additions & 9 deletions apps/dashboard/app/(app)/authorization/roles/create-new-role.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,26 +28,20 @@ import { trpc } from "@/lib/trpc/client";
import { zodResolver } from "@hookform/resolvers/zod";
import { DialogTrigger } from "@radix-ui/react-dialog";
import type { Permission } from "@unkey/db";
import { validation } from "@unkey/validation";
import { useRouter } from "next/navigation";
import { useState } from "react";
import { useForm } from "react-hook-form";
import { z } from "zod";

type Props = {
trigger: React.ReactNode;
permissions?: Permission[];
};

const formSchema = z.object({
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",
}),
name: validation.name,

description: z.string().optional(),
description: validation.description.optional(),
permissionOptions: z
.array(
z.object({
Expand Down
15 changes: 4 additions & 11 deletions apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/update-namespace-name.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,14 @@ import { toast } from "@/components/ui/toaster";
import { tags } from "@/lib/cache";
import { trpc } from "@/lib/trpc/client";
import { zodResolver } from "@hookform/resolvers/zod";
import { validation } from "@unkey/validation";
import { useRouter } from "next/navigation";
import { useForm } from "react-hook-form";
import { z } from "zod";
const formSchema = z.object({
name: z
.string()
.trim()
.min(3)
.max(50)
.regex(/^[a-zA-Z0-9_\-\.]+$/, {
message:
"Name must be 3-50 characters long and can only contain letters, numbers, underscores, hyphens, and periods.",
}),
namespaceId: z.string(),
workspaceId: z.string(),
name: validation.name,
namespaceId: validation.unkeyId,
workspaceId: validation.unkeyId,
});

type Props = {
Expand Down
1 change: 1 addition & 0 deletions apps/dashboard/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"@unkey/rbac": "workspace:^",
"@unkey/resend": "workspace:^",
"@unkey/schema": "workspace:^",
"@unkey/validation": "workspace:^",
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 18, 2024

Choose a reason for hiding this comment

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

💡 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

"@unkey/vault": "workspace:^",
"@unkey/vercel": "workspace:^",
"@upstash/ratelimit": "^2.0.1",
Expand Down
3 changes: 3 additions & 0 deletions internal/validation/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
`@unkey/validation` contains various shared schema or validation utils.

For example user-chosen identifiers.
18 changes: 18 additions & 0 deletions internal/validation/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"name": "@unkey/validation",
"version": "1.0.0",
"description": "",
"main": "src/index.ts",
"types": "src/index.ts",
"keywords": [],
"author": "Andreas Thomas",
"license": "AGPL-3.0",
"devDependencies": {
"@types/node": "^20.14.9",
"@unkey/tsconfig": "workspace:*",
"typescript": "^5.5.3"
},
"dependencies": {
"zod": "^3.23.5"
}
}
46 changes: 46 additions & 0 deletions internal/validation/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { z } from "zod";

/*
* An identifier is any string the user gives us to be used as a lookup key.
* It must be URL safe and fit into our database (varchar 256)
*/
const identifier = z
.string()
.min(3)
.max(256)
.regex(
/^[a-zA-Z0-9_\.:\-]*$/,
"Only alphanumeric, underscores, periods, colons and hyphens are allowed",
);

/**
* 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);
Comment on lines +16 to +23
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 18, 2024

Choose a reason for hiding this comment

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

🛠️ 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.

Suggested change
/**
* 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 /"
);


/**
* A description is a user given human-readable string.
*
* It must not be used in URLs.
*
* @example The description of a permission
*/
const description = z.string().min(3).max(256);

const unkeyId = z
.string()
.regex(
/^[a-z]{3,4}_[a-zA-Z0-9]{8,}$/,
"Unkey IDs must include a prefix, separated by an underscore: key_abcdefg123",
);

export const validation = {
identifier,
name,
description,
unkeyId,
};
8 changes: 8 additions & 0 deletions internal/validation/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"$schema": "https://json.schemastore.org/tsconfig",
"extends": "@unkey/tsconfig/base.json",
"exclude": ["dist"],
"compilerOptions": {
"outDir": "dist"
}
}
Loading