API: require auth and identity-bound ownership for POST /api/tasks (match docs contract)#414
Conversation
- Updated the import of the task validation function to `validateCreateTaskRequest` for clarity. - Modified the `createTaskHandler` to utilize the new validation function, which now includes JSON body validation and authentication context checks. - Added new tests for `createTaskHandler` and `validateCreateTaskRequest` to ensure proper functionality and error handling.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe validation logic for task creation is consolidated into a request-level validator that handles JSON parsing, body validation via Zod, and auth context resolution. The handler now delegates validation to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
lib/tasks/validateCreateTaskBody.ts (1)
73-102: Split request-level validation into its own file to preserve SRP and naming consistency.
validateCreateTaskRequestintroduces a second exported validator in a...Body.tsmodule and is >20 lines while handling multiple responsibilities (JSON parse, body validate, auth validate, account override). Please move it to a dedicated file (for example,validateCreateTaskRequest.ts) and keepvalidateCreateTaskBody.tsfocused on schema/body validation.As per coding guidelines:
lib/**/*.ts: "Apply Single Responsibility Principle (SRP): one exported function per file; each file should do one thing well",**/*.{js,ts,tsx,jsx}: "each file must export ONE primary function, and the file name must match that function", and**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php}: "Flag functions longer than 20 lines".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/tasks/validateCreateTaskBody.ts` around lines 73 - 102, The validateCreateTaskRequest function in validateCreateTaskBody.ts mixes JSON parsing, body validation, auth check and account override and must be moved to its own file; create a new file named e.g. validateCreateTaskRequest.ts exporting the single function validateCreateTaskRequest, have it call validateCreateTaskBody(body) for schema validation and validateAuthContext(request, { accountId: validatedBody.account_id }) for auth, reuse getCorsHeaders for the JSON parse error response, and return the merged object with account_id: auth.accountId; remove the validateCreateTaskRequest export from validateCreateTaskBody.ts so that file only exports validateCreateTaskBody.lib/tasks/createTaskHandler.ts (1)
21-53: Consider trimmingcreateTaskHandlerto keep it under the 20-line guideline and more focused.The handler currently mixes validation orchestration, persistence call, and response shaping. Extracting response builders (success/error) would keep this function tighter and easier to maintain.
As per coding guidelines:
**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php}: "Flag functions longer than 20 lines" andlib/**/*.ts: "Single responsibility per function".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/tasks/createTaskHandler.ts` around lines 21 - 53, createTaskHandler is doing validation, persistence and response shaping in one function; extract the success and error response builders into small helpers (e.g., buildCreateTaskSuccessResponse and buildCreateTaskErrorResponse) and move the NextResponse.json + getCorsHeaders logic into those helpers so createTaskHandler only orchestrates validateCreateTaskRequest and createTask and returns the helper results; keep existing behavior (return validated NextResponse early, call createTask with validatedBody, log error in the catch and pass error.message into the error builder) and reference createTaskHandler, validateCreateTaskRequest, createTask, getCorsHeaders when locating where to extract helpers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/tasks/createTaskHandler.ts`:
- Around line 21-53: createTaskHandler is doing validation, persistence and
response shaping in one function; extract the success and error response
builders into small helpers (e.g., buildCreateTaskSuccessResponse and
buildCreateTaskErrorResponse) and move the NextResponse.json + getCorsHeaders
logic into those helpers so createTaskHandler only orchestrates
validateCreateTaskRequest and createTask and returns the helper results; keep
existing behavior (return validated NextResponse early, call createTask with
validatedBody, log error in the catch and pass error.message into the error
builder) and reference createTaskHandler, validateCreateTaskRequest, createTask,
getCorsHeaders when locating where to extract helpers.
In `@lib/tasks/validateCreateTaskBody.ts`:
- Around line 73-102: The validateCreateTaskRequest function in
validateCreateTaskBody.ts mixes JSON parsing, body validation, auth check and
account override and must be moved to its own file; create a new file named e.g.
validateCreateTaskRequest.ts exporting the single function
validateCreateTaskRequest, have it call validateCreateTaskBody(body) for schema
validation and validateAuthContext(request, { accountId:
validatedBody.account_id }) for auth, reuse getCorsHeaders for the JSON parse
error response, and return the merged object with account_id: auth.accountId;
remove the validateCreateTaskRequest export from validateCreateTaskBody.ts so
that file only exports validateCreateTaskBody.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6dc5a055-e9da-4b62-b26a-fdc18441ea59
⛔ Files ignored due to path filters (2)
lib/tasks/__tests__/createTaskHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/tasks/__tests__/validateCreateTaskRequest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (2)
lib/tasks/createTaskHandler.tslib/tasks/validateCreateTaskBody.ts
There was a problem hiding this comment.
4 issues found across 4 files
Confidence score: 4/5
- This PR appears safe to merge with low functional risk; the reported issues are maintainability/style concerns rather than clear runtime bugs or regressions.
- The highest-severity findings are in
lib/tasks/validateCreateTaskBody.tsand indicate file-size/SRP convention drift, which can make future changes harder to reason about and review. - Test files
lib/tasks/__tests__/validateCreateTaskRequest.test.tsandlib/tasks/__tests__/createTaskHandler.test.tsexceed the 100-line project guideline, which may reduce readability but is not likely to break behavior today. - Pay close attention to
lib/tasks/validateCreateTaskBody.ts,lib/tasks/__tests__/validateCreateTaskRequest.test.ts, andlib/tasks/__tests__/createTaskHandler.test.ts- refactor for size/SRP alignment to keep maintenance risk from growing.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/tasks/__tests__/createTaskHandler.test.ts">
<violation number="1" location="lib/tasks/__tests__/createTaskHandler.test.ts:1">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
Newly added test file exceeds the rule’s 100-line maximum (120 lines), violating the maintainability size limit.</violation>
</file>
<file name="lib/tasks/__tests__/validateCreateTaskRequest.test.ts">
<violation number="1" location="lib/tasks/__tests__/validateCreateTaskRequest.test.ts:1">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
New test module exceeds the Rule 3 file-length limit (<100 lines), reducing readability and maintainability.</violation>
</file>
<file name="lib/tasks/validateCreateTaskBody.ts">
<violation number="1" location="lib/tasks/validateCreateTaskBody.ts:73">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
File exceeds the custom 100-line maintainability limit after PR additions.</violation>
<violation number="2" location="lib/tasks/validateCreateTaskBody.ts:73">
P2: Per the project's SRP convention, `validateCreateTaskRequest` should live in its own file `lib/tasks/validateCreateTaskRequest.ts`. The file-naming rule requires each file to export one primary function whose name matches the filename.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client
participant Handler as createTaskHandler
participant Validator as validateCreateTaskRequest
participant Auth as validateAuthContext
participant DB as createTask (DB Service)
Note over Client,DB: POST /api/tasks Flow
Client->>Handler: POST request (JSON + Auth Headers)
Handler->>Validator: NEW: validateCreateTaskRequest(request)
Validator->>Validator: Parse JSON body
alt Invalid JSON
Validator-->>Handler: 400 Bad Request
Handler-->>Client: Error response
end
Validator->>Validator: Zod schema validation
alt Schema validation fails
Validator-->>Handler: 400 Bad Request
Handler-->>Client: Error response
end
Validator->>Auth: NEW: validateAuthContext(request, { accountId })
Note right of Auth: Verifies API Key/JWT and<br/>identity-bound ownership
alt Auth or Ownership failure
Auth-->>Validator: 401 Unauthorized / 403 Forbidden
Validator-->>Handler: Error Response
Handler-->>Client: Error response
else Auth Success
Auth-->>Validator: Resolved Auth Context (accountId, etc.)
Validator-->>Handler: Validated CreateTaskBody
end
Handler->>DB: createTask(validatedBody)
alt DB Success
DB-->>Handler: Created Task Record
Handler-->>Client: 200 OK (Success + Task Data)
else DB Failure (Exception)
DB-->>Handler: Error
Handler-->>Client: 500 Internal Server Error
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| @@ -0,0 +1,120 @@ | |||
| import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; | |||
There was a problem hiding this comment.
P2: Custom agent: Enforce Clear Code Style and Maintainability Practices
Newly added test file exceeds the rule’s 100-line maximum (120 lines), violating the maintainability size limit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/tasks/__tests__/createTaskHandler.test.ts, line 1:
<comment>Newly added test file exceeds the rule’s 100-line maximum (120 lines), violating the maintainability size limit.</comment>
<file context>
@@ -0,0 +1,120 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+import { NextRequest, NextResponse } from "next/server";
+import { createTaskHandler } from "@/lib/tasks/createTaskHandler";
</file context>
| @@ -0,0 +1,257 @@ | |||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | |||
There was a problem hiding this comment.
P2: Custom agent: Enforce Clear Code Style and Maintainability Practices
New test module exceeds the Rule 3 file-length limit (<100 lines), reducing readability and maintainability.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/tasks/__tests__/validateCreateTaskRequest.test.ts, line 1:
<comment>New test module exceeds the Rule 3 file-length limit (<100 lines), reducing readability and maintainability.</comment>
<file context>
@@ -0,0 +1,257 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { NextRequest, NextResponse } from "next/server";
+import { validateCreateTaskRequest } from "@/lib/tasks/validateCreateTaskBody";
</file context>
| @@ -1,5 +1,6 @@ | |||
| import { NextResponse } from "next/server"; | |||
| import { NextRequest, NextResponse } from "next/server"; | |||
There was a problem hiding this comment.
P2: Custom agent: Enforce Clear Code Style and Maintainability Practices
File exceeds the custom 100-line maintainability limit after PR additions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/tasks/validateCreateTaskBody.ts, line 73:
<comment>File exceeds the custom 100-line maintainability limit after PR additions.</comment>
<file context>
@@ -62,3 +63,40 @@ export function validateCreateTaskBody(body: unknown): NextResponse | CreateTask
+ * @param request - The incoming Next.js request
+ * @returns Error response or {@link CreateTaskBody} with account_id set from resolved auth context
+ */
+export async function validateCreateTaskRequest(
+ request: NextRequest,
+): Promise<NextResponse | CreateTaskBody> {
</file context>
| * @param request - The incoming Next.js request | ||
| * @returns Error response or {@link CreateTaskBody} with account_id set from resolved auth context | ||
| */ | ||
| export async function validateCreateTaskRequest( |
There was a problem hiding this comment.
P2: Per the project's SRP convention, validateCreateTaskRequest should live in its own file lib/tasks/validateCreateTaskRequest.ts. The file-naming rule requires each file to export one primary function whose name matches the filename.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/tasks/validateCreateTaskBody.ts, line 73:
<comment>Per the project's SRP convention, `validateCreateTaskRequest` should live in its own file `lib/tasks/validateCreateTaskRequest.ts`. The file-naming rule requires each file to export one primary function whose name matches the filename.</comment>
<file context>
@@ -62,3 +63,40 @@ export function validateCreateTaskBody(body: unknown): NextResponse | CreateTask
+ * @param request - The incoming Next.js request
+ * @returns Error response or {@link CreateTaskBody} with account_id set from resolved auth context
+ */
+export async function validateCreateTaskRequest(
+ request: NextRequest,
+): Promise<NextResponse | CreateTaskBody> {
</file context>
| * @param body - The request body to validate. | ||
| * @returns A NextResponse with an error if validation fails, or the validated body if validation passes. | ||
| */ | ||
| export function validateCreateTaskBody(body: unknown): NextResponse | CreateTaskBody { |
There was a problem hiding this comment.
YAGNI principle
- is this validateCreateTaskBody function still being used?
validateCreateTaskRequestfor clarity.createTaskHandlerto utilize the new validation function, which now includes JSON body validation and authentication context checks.createTaskHandlerandvalidateCreateTaskRequestto ensure proper functionality and error handling.Summary by CodeRabbit