Skip to content

fix(api): Add missing zValidator for api endpoints#1076

Open
kalpadhwaryu wants to merge 9 commits intomainfrom
add-missing-zValidator
Open

fix(api): Add missing zValidator for api endpoints#1076
kalpadhwaryu wants to merge 9 commits intomainfrom
add-missing-zValidator

Conversation

@kalpadhwaryu
Copy link
Contributor

@kalpadhwaryu kalpadhwaryu commented Oct 9, 2025

Description

Add missing zValidator middleware for api endpoints.

Testing

Tested locally.

Summary by CodeRabbit

  • New Features

    • Broadly expanded runtime input validation across APIs: drive item queries, batch file uploads, attachments, file serving, collections and item operations, knowledge-base uploads, agent prompts, connector IDs, tuning jobs, test-email payloads, and user/workspace/API-key queries.
  • Refactor

    • Routes updated to consistently apply per-endpoint validation using the new schemas without changing runtime behavior.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 9, 2025

Walkthrough

Adds many exported Zod validation schemas across server API modules and types, consolidates a DuplicateStrategy enum in knowledgeBase, and wires these schemas into server route registrations with zValidator. Changes are declarations and route wiring; runtime logic remains unchanged.

Changes

Cohort / File(s) Summary of changes
Admin param schema
server/api/admin.ts
Adds exported connectorIdParamsSchema for validating connectorId as non-empty string and wires it to admin connector routes.
Agent params & queries
server/api/agent.ts
Adds exported getAgentParamsSchema and generatePromptQuerySchema and minor formatting tweak in an existing query.
DataSource params
server/api/dataSource.ts
Adds exported getDataSourceFileParamsSchema, listDataSourceFilesParamsSchema, getAgentsForDataSourceParamsSchema for basic param validation.
Search params
server/api/search.ts
Adds exported getDriveItemSchema and getDriveItemsByDocIdsSchema; minor refactor of messageSchema transform.
Test email schema
server/api/testEmail.ts
Adds exported sendEmailSchema (email, optional subject/body); minor SES logging formatting change.
Tuning params
server/api/tuning.ts
Adds exported deleteDatasetParamsSchema and tuningWsParamsSchema.
Knowledge Base schemas
server/api/knowledgeBase.ts
Moves DuplicateStrategy enum to top, removes duplicate, and introduces many exported schemas (collections, items, file ops, chunking, uploads, polling) like pollCollectionsStatusSchema, collectionParamsSchema, uploadFilesFormSchema, etc.
Types / shared schemas
server/types.ts
Adds exported schemas/types for file/attachment uploads and related params: fileUploadSchema/FileUpload, attachmentUploadSchema/AttachmentUpload, fileServeParamsSchema/FileServeParams, getUserWorkspaceInfoQuerySchema/GetUserWorkspaceInfoQuery, deleteUserApiKeyParamsSchema/DeleteUserApiKeyParams.
Server route integrations
server/server.ts
Imports new schemas and wraps many routes with zValidator for params/query/json/form (search, KB, admin, agent, tuning, dataSource, email, user endpoints); updates public exports to include new schemas.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant R as Router
  participant V as zValidator
  participant H as Handler
  participant S as Service/DB

  C->>R: HTTP request (path/params/query/form/json)
  R->>V: Validate using route schema (new/expanded Zod schemas)
  alt valid
    V-->>R: typed input
    R->>H: call handler with validated data
    H->>S: perform operation
    S-->>H: result
    H-->>C: 200 + response
  else invalid
    V-->>C: 400 Validation error
  end

  rect rgba(100,150,200,0.08)
    note right of V: Many new schemas added across modules (search, KB, agent, dataSource, tuning, admin, testEmail, types)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Potential areas to spot-check:

  • server/server.ts route wiring for correct schema used per route (params vs query vs body/form).
  • server/api/knowledgeBase.ts enum relocation and removal of duplicate definition.
  • Transform/regex in deleteUserApiKeyParamsSchema (type coercion) and any optional/required mismatch in upload schemas.

Possibly related PRs

Suggested reviewers

  • zereraz
  • shivamashtikar
  • junaid-shirur
  • devesh-juspay

Poem

A hop, I added schemas bright,
I guard each param through day and night.
Zod snug fences, inputs kept neat,
Routes now dance with typed-with-feet.
Carrots for tests — a tidy treat! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "fix(api): Add missing zValidator for api endpoints" is clear and directly related to the changeset. The PR's primary objective is to add zValidator middleware across API endpoints by creating validation schemas in multiple API files (admin, agent, dataSource, knowledgeBase, search, testEmail, tuning, and types) and integrating them into routes in server.ts. The title uses specific terminology ("zValidator" and "api endpoints") rather than vague language, making it clear to reviewers scanning the history that the change concerns validation middleware additions. The title encompasses the main change without needing to detail every file or schema involved.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add-missing-zValidator

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
server/api/tuning.ts (1)

1503-1535: Use c.req.valid("json") instead of re-reading the body

Once the route is wrapped with zValidator("json", evaluateSchema), the request stream has already been consumed. Calling c.req.json() again now resolves to null (due to the catch), so any client-supplied numSamples is silently ignored. Reuse the validated payload from the validator, e.g. const body = c.req.valid("json") as z.infer<typeof evaluateSchema>, so the handler continues to respect the parameter.

-    const body = await c.req.json().catch(() => null)
-    if (body) {
-      const validation = evaluateSchema.safeParse(body)
-      if (!validation.success) {
-        return c.json(
-          { error: "Invalid request body", details: validation.error },
-          400,
-        )
-      }
-      numSamples = validation.data.numSamples
-      loggerWithChild({ email: userEmail }).info(
-        { userEmail, numSamples },
-        "Received evaluate request with parameters",
-      )
-    } else {
-      loggerWithChild({ email: userEmail }).info(
-        { userEmail },
-        "Received evaluate request with empty/invalid body, using defaults.",
-      )
-    }
+    const body = c.req.valid("json") as z.infer<typeof evaluateSchema>
+    numSamples = body.numSamples
+    if (Object.keys(body).length > 0) {
+      loggerWithChild({ email: userEmail }).info(
+        { userEmail, numSamples },
+        "Received evaluate request with parameters",
+      )
+    } else {
+      loggerWithChild({ email: userEmail }).info(
+        { userEmail },
+        "Received evaluate request with empty body, using defaults.",
+      )
+    }
server/api/search.ts (1)

731-776: Avoid re-reading the JSON body after zValidator

Now that /search/driveitem and /search/driveitemsbydocids run through zValidator("json", …), the request stream is consumed before the handler executes. Calling await c.req.json() here throws TypeError: body used already for: ReadableStream, breaking the endpoint. Instead, grab the already-validated payload via c.req.valid("json").

-  const body = await c.req.json()
-  const { parentId } = body
+  const { parentId } = c.req.valid("json") as z.infer<typeof getDriveItemSchema>
@@
-  const body = await c.req.json()
-  const { docIds } = body
+  const { docIds } = c.req.valid("json") as z.infer<
+    typeof getDriveItemsByDocIdsSchema
+  >
🧹 Nitpick comments (2)
server/api/knowledgeBase.ts (2)

156-184: Consider consolidating duplicate schemas.

Several schemas validate identical parameters:

  • collectionParamsSchema, collectionNameForSharedAgentParamsSchema, and listCollectionItemsParamsSchema all validate { clId: string }
  • deleteItemParamsSchema and fileOperationParamsSchema both validate { clId: string, itemId: string }

While separate schemas can provide better type inference and future flexibility, consolidating them would follow DRY principles and reduce maintenance overhead.

Consider this refactor:

 const collectionParamsSchema = z.object({
   clId: z.string().min(1),
 })

-const collectionNameForSharedAgentParamsSchema = z.object({
-  clId: z.string().min(1),
-})
-
-const listCollectionItemsParamsSchema = z.object({
-  clId: z.string().min(1),
-})
+// Reuse collectionParamsSchema for these endpoints
+const collectionNameForSharedAgentParamsSchema = collectionParamsSchema
+const listCollectionItemsParamsSchema = collectionParamsSchema

Similarly for the item operation schemas:

 const deleteItemParamsSchema = z.object({
   clId: z.string().min(1),
   itemId: z.string().min(1),
 })

-const fileOperationParamsSchema = z.object({
-  clId: z.string().min(1),
-  itemId: z.string().min(1),
-})
+// Reuse deleteItemParamsSchema for file operations
+const fileOperationParamsSchema = deleteItemParamsSchema

191-194: Consider using boolean coercion for query parameters.

The schema defines ownOnly and includeItems as strings, which are then compared to "true" in the handler (Lines 439-440). While this works, using z.coerce.boolean() would provide better type safety and cleaner handler code.

Apply this diff:

 const listCollectionsQuerySchema = z.object({
-  ownOnly: z.string().optional(),
-  includeItems: z.string().optional(),
+  ownOnly: z.coerce.boolean().optional(),
+  includeItems: z.coerce.boolean().optional(),
 })

This would allow the handler to use the boolean values directly instead of string comparisons:

-const showOnlyOwn = c.req.query("ownOnly") === "true"
-const includeItems = c.req.query("includeItems") === "true"
+const showOnlyOwn = validatedQuery.ownOnly ?? false
+const includeItems = validatedQuery.includeItems ?? false
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aa29eda and 883284f.

📒 Files selected for processing (9)
  • server/api/admin.ts (1 hunks)
  • server/api/agent.ts (2 hunks)
  • server/api/dataSource.ts (1 hunks)
  • server/api/knowledgeBase.ts (2 hunks)
  • server/api/search.ts (3 hunks)
  • server/api/testEmail.ts (1 hunks)
  • server/api/tuning.ts (1 hunks)
  • server/server.ts (15 hunks)
  • server/types.ts (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
server/api/agent.ts (1)
server/db/schema/users.ts (1)
  • users (27-57)
server/api/testEmail.ts (1)
server/logger/index.ts (2)
  • getLogger (36-93)
  • Subsystem (15-15)
server/server.ts (8)
server/types.ts (5)
  • fileUploadSchema (635-643)
  • attachmentUploadSchema (647-649)
  • fileServeParamsSchema (653-655)
  • getUserWorkspaceInfoQuerySchema (660-662)
  • deleteUserApiKeyParamsSchema (670-675)
server/api/files.ts (5)
  • handleFileUpload (45-195)
  • handleAttachmentUpload (197-407)
  • handleAttachmentServe (554-621)
  • handleThumbnailServe (626-673)
  • handleAttachmentDeleteApi (515-549)
server/api/search.ts (3)
  • handleAttachmentDeleteSchema (269-271)
  • getDriveItemSchema (261-263)
  • getDriveItemsByDocIdsSchema (265-267)
server/api/dataSource.ts (6)
  • getDataSourceFileParamsSchema (235-237)
  • GetDataSourceFile (553-715)
  • listDataSourceFilesParamsSchema (239-241)
  • ListDataSourceFilesApi (500-551)
  • getAgentsForDataSourceParamsSchema (243-245)
  • GetAgentsForDataSourceApi (437-460)
server/api/tuning.ts (3)
  • evaluateSchema (200-204)
  • deleteDatasetParamsSchema (207-209)
  • tuningWsParamsSchema (212-214)
server/api/agent.ts (5)
  • generatePromptQuerySchema (128-131)
  • getAgentParamsSchema (124-126)
  • GetAgentApi (75-115)
  • GetAgentPermissionsApi (534-586)
  • GetAgentIntegrationItemsApi (588-751)
server/api/testEmail.ts (1)
  • sendEmailSchema (9-13)
server/api/knowledgeBase.ts (29)
  • createCollectionSchema (214-214)
  • CreateCollectionApi (328-423)
  • listCollectionsQuerySchema (226-226)
  • ListCollectionsApi (426-503)
  • pollCollectionsStatusSchema (217-217)
  • PollCollectionsStatusApi (1960-2004)
  • collectionParamsSchema (218-218)
  • GetCollectionApi (506-543)
  • collectionNameForSharedAgentParamsSchema (219-219)
  • collectionNameForSharedAgentQuerySchema (220-220)
  • GetCollectionNameForSharedAgentApi (545-609)
  • updateCollectionSchema (215-215)
  • UpdateCollectionApi (612-667)
  • DeleteCollectionApi (670-846)
  • listCollectionItemsParamsSchema (221-221)
  • listCollectionItemsQuerySchema (222-222)
  • ListCollectionItemsApi (849-888)
  • createFolderSchema (216-216)
  • CreateFolderApi (891-982)
  • uploadFilesFormSchema (227-227)
  • UploadFilesApi (1126-1560)
  • deleteItemParamsSchema (223-223)
  • DeleteItemApi (1563-1758)
  • fileOperationParamsSchema (224-224)
  • GetFilePreviewApi (1761-1835)
  • GetFileContentApi (1920-1957)
  • DownloadFileApi (2007-2243)
  • chunkContentParamsSchema (225-225)
  • GetChunkContentApi (1837-1917)
server/types.ts (1)
frontend/src/components/FileUpload.tsx (1)
  • FileUpload (36-698)
🔇 Additional comments (2)
server/api/knowledgeBase.ts (2)

124-129: LGTM! Good consolidation of the enum.

Moving the DuplicateStrategy enum to the top improves code organization by positioning it before its first usage in the schemas.


213-228: LGTM! Schema exports are complete and well-organized.

All schemas, including the newly added validation schemas, are properly exported for use in the server route registrations.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
server/api/knowledgeBase.ts (1)

187-190: Address the cId validation issue flagged in previous reviews.

This concern was already raised in a previous review: the schema validates cId as any non-empty string, but Line 1840 parses it as an integer with parseInt(c.req.param("cId")), which will result in NaN for non-numeric input.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 883284f and a9e5b19.

📒 Files selected for processing (5)
  • server/api/admin.ts (1 hunks)
  • server/api/knowledgeBase.ts (2 hunks)
  • server/api/search.ts (3 hunks)
  • server/server.ts (14 hunks)
  • server/types.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • server/api/search.ts
  • server/api/admin.ts
🧰 Additional context used
🧬 Code graph analysis (2)
server/server.ts (10)
server/types.ts (5)
  • fileUploadSchema (639-647)
  • attachmentUploadSchema (651-653)
  • fileServeParamsSchema (657-659)
  • getUserWorkspaceInfoQuerySchema (664-666)
  • deleteUserApiKeyParamsSchema (674-679)
server/api/files.ts (5)
  • handleFileUpload (45-195)
  • handleAttachmentUpload (197-407)
  • handleAttachmentServe (554-621)
  • handleThumbnailServe (626-673)
  • handleAttachmentDeleteApi (515-549)
server/api/search.ts (5)
  • handleAttachmentDeleteSchema (269-271)
  • getDriveItemSchema (261-263)
  • GetDriveItem (730-755)
  • getDriveItemsByDocIdsSchema (265-267)
  • GetDriveItemsByDocIds (757-783)
server/api/auth.ts (1)
  • GetUserWorkspaceInfo (22-46)
server/api/dataSource.ts (6)
  • getDataSourceFileParamsSchema (235-237)
  • GetDataSourceFile (553-715)
  • listDataSourceFilesParamsSchema (239-241)
  • ListDataSourceFilesApi (500-551)
  • getAgentsForDataSourceParamsSchema (243-245)
  • GetAgentsForDataSourceApi (437-460)
server/api/tuning.ts (4)
  • evaluateSchema (200-204)
  • EvaluateHandler (1503-1554)
  • deleteDatasetParamsSchema (207-209)
  • tuningWsParamsSchema (212-214)
server/api/agent.ts (5)
  • generatePromptQuerySchema (128-131)
  • getAgentParamsSchema (124-126)
  • GetAgentApi (75-115)
  • GetAgentPermissionsApi (534-586)
  • GetAgentIntegrationItemsApi (588-751)
server/api/testEmail.ts (2)
  • sendEmailSchema (9-13)
  • sendMailHelper (15-47)
server/api/knowledgeBase.ts (28)
  • createCollectionSchema (215-215)
  • CreateCollectionApi (329-424)
  • listCollectionsQuerySchema (227-227)
  • ListCollectionsApi (427-504)
  • pollCollectionsStatusSchema (218-218)
  • PollCollectionsStatusApi (1967-2011)
  • collectionParamsSchema (219-219)
  • GetCollectionApi (507-544)
  • collectionNameForSharedAgentParamsSchema (220-220)
  • collectionNameForSharedAgentQuerySchema (221-221)
  • GetCollectionNameForSharedAgentApi (546-610)
  • updateCollectionSchema (216-216)
  • UpdateCollectionApi (613-668)
  • listCollectionItemsParamsSchema (222-222)
  • listCollectionItemsQuerySchema (223-223)
  • ListCollectionItemsApi (850-889)
  • createFolderSchema (217-217)
  • CreateFolderApi (892-983)
  • uploadFilesFormSchema (228-228)
  • UploadFilesApi (1127-1561)
  • deleteItemParamsSchema (224-224)
  • DeleteItemApi (1564-1759)
  • fileOperationParamsSchema (225-225)
  • GetFilePreviewApi (1762-1836)
  • GetFileContentApi (1927-1964)
  • DownloadFileApi (2014-2250)
  • chunkContentParamsSchema (226-226)
  • GetChunkContentApi (1838-1924)
server/api/admin.ts (2)
  • connectorIdParamsSchema (208-210)
  • GetConnectorTools (235-277)
server/types.ts (1)
frontend/src/components/FileUpload.tsx (1)
  • FileUpload (36-698)
🔇 Additional comments (6)
server/api/knowledgeBase.ts (2)

125-130: LGTM! Enum consolidation improves organization.

Moving the DuplicateStrategy enum to the top scope and removing the duplicate definition enhances code organization and prevents potential inconsistencies.


214-229: LGTM! Schema exports enable broader validation coverage.

Exporting the schemas allows them to be imported and used in server/server.ts for route-level validation, which aligns well with the PR's objective of adding missing zValidator middleware.

server/types.ts (1)

674-679: LGTM! Excellent validation pattern for numeric path parameters.

The deleteUserApiKeyParamsSchema correctly validates that keyId is a numeric string using a regex, then transforms it to a number. This prevents NaN issues and provides clear error messages to clients.

server/server.ts (3)

925-945: LGTM! Comprehensive validation for file upload routes.

All file and attachment handling routes are properly validated with appropriate schemas. The use of zValidator for both form data and path parameters ensures runtime type safety.


1049-1063: LGTM! Datasource routes are properly validated.

All datasource endpoints correctly use zValidator with appropriate parameter schemas to ensure runtime type safety.


920-1592: Excellent validation coverage across the API surface.

The PR successfully adds comprehensive runtime validation using zValidator middleware across multiple API modules (admin, agent, datasource, KB, search, tuning, auth, email, workflow). This enhances type safety and provides clear error messages to API consumers.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
server/api/knowledgeBase.ts (1)

188-191: Validate numeric string for cId parameter.

The schema validates cId as any non-empty string, but Line 1969 parses it as an integer with parseInt(c.req.param("cId")). If a non-numeric string is passed, this will result in NaN and potential runtime errors.

Apply this diff to ensure cId is a valid numeric string:

 const chunkContentParamsSchema = z.object({
-  cId: z.string().min(1),
+  cId: z.string().regex(/^\d+$/, "cId must be a valid integer"),
   docId: z.string().min(1),
 })

Alternatively, use z.coerce.number().int().nonnegative() to automatically parse and validate:

 const chunkContentParamsSchema = z.object({
-  cId: z.string().min(1),
+  cId: z.coerce.number().int().nonnegative(),
   docId: z.string().min(1),
 })

If you choose coercion, update the handler at Line 1969 to use c.req.param("cId") directly without parseInt.

server/server.ts (1)

1330-1331: Add missing validators for batch upload and complete endpoints.

Both routes share the UploadFilesApi handler with the main upload route but lack validation middleware:

  1. Line 1330 (batch upload): Should validate the same path parameters and form data as the main upload route at lines 1324-1329
  2. Line 1331 (complete): The handler expects JSON body with sessionId (see handler line 1224: const { sessionId } = await c.req.json()), but the route has no validator

Apply this diff:

- .post("/cl/:clId/items/upload/batch", UploadFilesApi) // Batch upload endpoint
- .post("/cl/:clId/items/upload/complete", UploadFilesApi) // Complete batch session
+ .post(
+   "/cl/:clId/items/upload/batch",
+   zValidator("param", collectionParamsSchema),
+   zValidator("form", uploadFilesFormSchema),
+   UploadFilesApi,
+ )
+ .post(
+   "/cl/:clId/items/upload/complete",
+   zValidator("param", collectionParamsSchema),
+   zValidator("json", z.object({
+     sessionId: z.string().min(1, "Session ID is required"),
+   })),
+   UploadFilesApi,
+ )

Note: The complete endpoint schema should be defined at the module level (in knowledgeBase.ts) and exported for consistency with other schemas.

🧹 Nitpick comments (1)
server/api/knowledgeBase.ts (1)

193-196: Consider boolean coercion for query parameters.

The schema validates ownOnly and includeItems as optional strings, requiring manual === "true" checks in the handler (lines 472-473). Consider using Zod's boolean coercion pattern for cleaner validation:

Apply this diff to automatically coerce string query params to booleans:

 const listCollectionsQuerySchema = z.object({
-  ownOnly: z.string().optional(),
-  includeItems: z.string().optional(),
+  ownOnly: z.preprocess((val) => val === "true", z.boolean()).optional(),
+  includeItems: z.preprocess((val) => val === "true", z.boolean()).optional(),
 })

Then simplify the handler:

-  const showOnlyOwn = c.req.query("ownOnly") === "true"
-  const includeItems = c.req.query("includeItems") === "true"
+  const validatedQuery = c.req.valid("query")
+  const showOnlyOwn = validatedQuery.ownOnly ?? false
+  const includeItems = validatedQuery.includeItems ?? false
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a9e5b19 and 88a1381.

📒 Files selected for processing (5)
  • server/api/admin.ts (1 hunks)
  • server/api/agent.ts (2 hunks)
  • server/api/knowledgeBase.ts (2 hunks)
  • server/server.ts (14 hunks)
  • server/types.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • server/api/admin.ts
  • server/api/agent.ts
🧰 Additional context used
🧬 Code graph analysis (2)
server/types.ts (1)
frontend/src/components/FileUpload.tsx (1)
  • FileUpload (36-698)
server/server.ts (9)
server/types.ts (5)
  • fileUploadSchema (659-667)
  • attachmentUploadSchema (671-673)
  • fileServeParamsSchema (677-679)
  • getUserWorkspaceInfoQuerySchema (684-686)
  • deleteUserApiKeyParamsSchema (694-699)
server/api/dataSource.ts (3)
  • getDataSourceFileParamsSchema (235-237)
  • listDataSourceFilesParamsSchema (239-241)
  • getAgentsForDataSourceParamsSchema (243-245)
server/api/search.ts (2)
  • getDriveItemSchema (261-263)
  • getDriveItemsByDocIdsSchema (265-267)
server/api/tuning.ts (3)
  • evaluateSchema (200-204)
  • deleteDatasetParamsSchema (207-209)
  • tuningWsParamsSchema (212-214)
server/api/agent.ts (2)
  • generatePromptQuerySchema (138-141)
  • getAgentParamsSchema (134-136)
server/api/agent/promptGeneration.ts (1)
  • GeneratePromptApi (19-112)
server/api/testEmail.ts (1)
  • sendEmailSchema (9-13)
server/api/knowledgeBase.ts (16)
  • createCollectionSchema (216-216)
  • listCollectionsQuerySchema (228-228)
  • pollCollectionsStatusSchema (219-219)
  • collectionParamsSchema (220-220)
  • collectionNameForSharedAgentParamsSchema (221-221)
  • collectionNameForSharedAgentQuerySchema (222-222)
  • updateCollectionSchema (217-217)
  • listCollectionItemsParamsSchema (223-223)
  • listCollectionItemsQuerySchema (224-224)
  • ListCollectionItemsApi (905-944)
  • createFolderSchema (218-218)
  • uploadFilesFormSchema (229-229)
  • UploadFilesApi (1182-1690)
  • deleteItemParamsSchema (225-225)
  • fileOperationParamsSchema (226-226)
  • chunkContentParamsSchema (227-227)
server/api/admin.ts (1)
  • connectorIdParamsSchema (248-250)
🔇 Additional comments (8)
server/api/knowledgeBase.ts (2)

126-131: LGTM - Good consolidation.

The DuplicateStrategy enum is well-placed at the module top level and provides clear, self-documenting values for duplicate file handling. The consolidation mentioned in the summary eliminates code duplication.


198-230: LGTM - Appropriate form validation strategy.

The uploadFilesFormSchema correctly validates the structured form fields while delegating file and path validation to the handler. This is appropriate because:

  • File objects require size, MIME type, and binary content validation that Zod cannot effectively handle
  • Path arrays from FormData need custom processing logic

The comprehensive export block properly exposes all schemas for route wiring in server.ts.

server/types.ts (3)

659-667: LGTM - Appropriate file upload validation.

The schema correctly uses z.any() for the file field, which is appropriate because:

  • Binary file content, size limits, and MIME type validation must occur in the handler with access to the actual file buffer
  • Zod cannot effectively validate file objects at the schema level

The remaining fields properly validate the upload metadata and align with the frontend FileUpload component's FormData structure (as seen in the relevant code snippets).


671-679: LGTM - Schemas appropriately structured.

Both schemas are correctly implemented:

  • attachmentUploadSchema follows the same pattern as fileUploadSchema with z.any() for binary content
  • fileServeParamsSchema provides straightforward parameter validation for file serving endpoints

683-699: LGTM - Excellent numeric validation pattern.

The deleteUserApiKeyParamsSchema demonstrates a robust approach for validating numeric parameters:

  1. Uses regex to ensure the string contains only digits
  2. Provides a clear error message for invalid input
  3. Transforms the validated string to a number

This pattern prevents the NaN issues that can occur with z.coerce.number() and is preferable to simple string validation followed by manual parsing. Consider using this pattern for other numeric parameters like the cId issue in server/api/knowledgeBase.ts.

server/server.ts (3)

958-973: LGTM - File upload routes properly validated.

All file upload and serving routes correctly apply validators:

  • Form validators for POST endpoints with FormData
  • Param validators for GET endpoints with URL parameters

The validator usage matches the handler expectations and request types.


1203-1268: LGTM - Agent routes consistently validated.

All agent routes properly apply validators matching their request types:

  • Param validators for route parameters (:agentExternalId)
  • Query validators for query strings (generate-prompt)
  • JSON validators for request bodies

The validation pattern is consistent across all agent operations.


1082-1130: LGTM - Data source and tuning routes properly validated.

All routes in this section correctly apply validators:

  • Data source routes use param validators for URL parameters
  • Drive item routes use JSON validators for POST request bodies
  • Tuning routes use appropriate validators based on request type (JSON for POST, params for DELETE/GET)

The validation aligns with the handler expectations.

Comment on lines 1632 to 1646
.get("/agent/:agentExternalId", GetAgentApi) // Get Agent details
.get("/chat/history", zValidator("query", chatHistorySchema), ChatHistory) // List chat history
.post("/cl", CreateCollectionApi) // Create collection (KB)
.get("/cl", ListCollectionsApi) // List all collections
.post("/cl", zValidator("json", createCollectionSchema), CreateCollectionApi) // Create collection (KB)
.get(
"/cl",
zValidator("query", listCollectionsQuerySchema),
ListCollectionsApi,
) // List all collections
.get(
"/cl/search",
zValidator("query", searchKnowledgeBaseSchema), // Search over KB
SearchKnowledgeBaseApi,
)
.get("/cl/:clId", GetCollectionApi) // Get collection by ID
.put("/cl/:clId", UpdateCollectionApi) // Update collection (rename, etc.)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add missing validators for consumer collection routes.

Lines 1645-1646 are missing validators while similar routes have them:

  • Line 1634 (POST /cl) has createCollectionSchema validator
  • Line 1650 (DELETE /cl/:clId) has collectionParamsSchema validator

Lines 1645-1646 should also validate their parameters:

Apply this diff:

- .get("/cl/:clId", GetCollectionApi) // Get collection by ID
- .put("/cl/:clId", UpdateCollectionApi) // Update collection (rename, etc.)
+ .get(
+   "/cl/:clId",
+   zValidator("param", collectionParamsSchema),
+   GetCollectionApi,
+ )
+ .put(
+   "/cl/:clId",
+   zValidator("param", collectionParamsSchema),
+   zValidator("json", updateCollectionSchema),
+   UpdateCollectionApi,
+ )

This ensures consistency with the authenticated routes (lines 1290-1306) and validates both the path parameter and request body for the PUT endpoint.

🤖 Prompt for AI Agents
In server/server.ts around lines 1632-1646 the consumer collection routes for
GET /cl/:clId and PUT /cl/:clId are missing request validators; add
zValidator("params", collectionParamsSchema) to the GET /cl/:clId route and add
both zValidator("params", collectionParamsSchema) and zValidator("json",
updateCollectionSchema) to the PUT /cl/:clId route (and ensure the schemas are
imported) so path parameters and request body are validated consistently with
other collection routes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant