feat: secure account update endpoint#404
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 19 minutes and 1 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefactors PATCH /api/accounts to replace body-only validation with request-level validation that parses JSON, validates the body schema, enforces auth context via validateAuthContext, and passes a ValidatedUpdateAccountRequest into updateAccountHandler. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Route as "PATCH /api/accounts"
participant ReqValidator as validateUpdateAccountRequest
participant AuthValidator as validateAuthContext
participant Handler as updateAccountHandler
participant Response as NextResponse
Client->>Route: PATCH request
Route->>ReqValidator: validateUpdateAccountRequest(req)
ReqValidator->>ReqValidator: safeParseJson + schema validation
alt schema invalid
ReqValidator->>Response: 400 error + CORS headers
Response-->>Client: Error response
else schema valid
ReqValidator->>AuthValidator: validateAuthContext(req, accountId)
alt auth invalid
AuthValidator-->>ReqValidator: NextResponse (error)
ReqValidator->>Response: Auth error response
Response-->>Client: Error response
else auth valid
AuthValidator-->>ReqValidator: Auth context (normalized accountId)
ReqValidator->>Handler: Pass ValidatedUpdateAccountRequest
Handler->>Response: Process update & return response
Response-->>Client: Success response
end
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lib/accounts/validateUpdateAccountRequest.ts (1)
30-32: Validation module naming should follow the body/query convention.Please keep Zod schema/type exports in a
validate<EndpointName>Body.ts(orQuery.ts) file, and keep request-level orchestration/auth composition separate if needed.Based on learnings: "Applies to lib/**/validate*.ts : Create validate functions in
validate<EndpointName>Body.tsorvalidate<EndpointName>Query.tsfiles that export both the schema and inferred TypeScript type."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/accounts/validateUpdateAccountRequest.ts` around lines 30 - 32, The current validateUpdateAccountRequest function mixes request-level orchestration with the Zod schema/type exports; extract the Zod schema and inferred TypeScript type into a new module named validateUpdateAccountRequestBody.ts (export both the schema and the inferred type, e.g., UpdateAccountRequestSchema and UpdateAccountRequest), then update validateUpdateAccountRequest to import that schema/type and keep only the request-level logic (auth/composition/NextRequest→NextResponse handling) in this file; ensure validateUpdateAccountRequest references the imported UpdateAccountRequest type and schema names when parsing/validating.lib/accounts/updateAccountHandler.ts (1)
16-80:updateAccountHandleris carrying too many responsibilities in one function.It currently mixes existence checks, account update, account_info upsert, post-fetch, and error-to-response mapping. Please split into focused helpers to reduce branching and keep this handler within size limits.
As per coding guidelines: "Apply Single Responsibility Principle (SRP): one exported function per file; each file should do one thing well", "Keep functions under 50 lines", and "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/accounts/updateAccountHandler.ts` around lines 16 - 80, The handler updateAccountHandler is doing too much; split it into small helpers: add verifyAccountExists(accountId) that calls getAccountWithDetails and returns/throws if missing, applyNameUpdate(accountId, name) that calls updateAccount only when name !== undefined, and upsertAccountInfo(accountId, info) that uses selectAccountInfo, insertAccountInfo, and updateAccountInfo to perform the create-or-update logic for organization, image, instruction, jobTitle, roleType, companyName, and knowledges; keep the handler to orchestration only (call verifyAccountExists, applyNameUpdate, upsertAccountInfo, then fetch updated via getAccountWithDetails) and centralize response/error mapping to use getCorsHeaders and the existing error message logic. Ensure each new helper is small (<20–50 lines) and exported/placed in this file so the exported function per file rule holds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/accounts/validateUpdateAccountRequest.ts`:
- Around line 13-15: The schema currently requires accountId; change
updateAccountRequestSchema so accountId is optional (e.g.,
z.string().uuid().optional()) so the PATCH body no longer mandates it, and
ensure route handlers derive the account ID from authentication using
validateAuthContext() and only accept an accountId override when explicit
authorization logic permits it; update any code paths that consumed a required
accountId to use the auth-derived value (or the optional override) instead.
---
Nitpick comments:
In `@lib/accounts/updateAccountHandler.ts`:
- Around line 16-80: The handler updateAccountHandler is doing too much; split
it into small helpers: add verifyAccountExists(accountId) that calls
getAccountWithDetails and returns/throws if missing, applyNameUpdate(accountId,
name) that calls updateAccount only when name !== undefined, and
upsertAccountInfo(accountId, info) that uses selectAccountInfo,
insertAccountInfo, and updateAccountInfo to perform the create-or-update logic
for organization, image, instruction, jobTitle, roleType, companyName, and
knowledges; keep the handler to orchestration only (call verifyAccountExists,
applyNameUpdate, upsertAccountInfo, then fetch updated via
getAccountWithDetails) and centralize response/error mapping to use
getCorsHeaders and the existing error message logic. Ensure each new helper is
small (<20–50 lines) and exported/placed in this file so the exported function
per file rule holds.
In `@lib/accounts/validateUpdateAccountRequest.ts`:
- Around line 30-32: The current validateUpdateAccountRequest function mixes
request-level orchestration with the Zod schema/type exports; extract the Zod
schema and inferred TypeScript type into a new module named
validateUpdateAccountRequestBody.ts (export both the schema and the inferred
type, e.g., UpdateAccountRequestSchema and UpdateAccountRequest), then update
validateUpdateAccountRequest to import that schema/type and keep only the
request-level logic (auth/composition/NextRequest→NextResponse handling) in this
file; ensure validateUpdateAccountRequest references the imported
UpdateAccountRequest type and schema names when parsing/validating.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2ec649f1-776b-420f-b086-24b0c8d7af76
⛔ Files ignored due to path filters (1)
lib/accounts/__tests__/validateUpdateAccountRequest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (4)
app/api/accounts/route.tslib/accounts/updateAccountHandler.tslib/accounts/validateUpdateAccountBody.tslib/accounts/validateUpdateAccountRequest.ts
💤 Files with no reviewable changes (1)
- lib/accounts/validateUpdateAccountBody.ts
| export const updateAccountRequestSchema = z.object({ | ||
| accountId: z.string().uuid("accountId must be a valid UUID"), | ||
| name: z.string().optional(), |
There was a problem hiding this comment.
Avoid making accountId mandatory in the PATCH body.
This should be derived from authentication by default; keep override optional only when explicitly authorized. Requiring it in every request weakens the endpoint contract and increases misuse risk.
As per coding guidelines: "Never use account_id in request bodies or tool schemas; always derive the account ID from authentication" and "Always use validateAuthContext() for authentication in API routes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/accounts/validateUpdateAccountRequest.ts` around lines 13 - 15, The
schema currently requires accountId; change updateAccountRequestSchema so
accountId is optional (e.g., z.string().uuid().optional()) so the PATCH body no
longer mandates it, and ensure route handlers derive the account ID from
authentication using validateAuthContext() and only accept an accountId override
when explicit authorization logic permits it; update any code paths that
consumed a required accountId to use the auth-derived value (or the optional
override) instead.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 5 files
Confidence score: 4/5
- This PR is likely safe to merge, but there is a moderate validation gap in
lib/accounts/validateUpdateAccountRequest.tswhere a request containing onlyaccountIdis treated as a valid update. - The impact appears limited to request correctness (accepting no-op updates) rather than a clear crash or data-loss path, which keeps overall merge risk on the lower side.
- Pay close attention to
lib/accounts/validateUpdateAccountRequest.ts- enforce that at least one updatable field (in addition toaccountId) is required.
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/accounts/validateUpdateAccountRequest.ts">
<violation number="1" location="lib/accounts/validateUpdateAccountRequest.ts:13">
P2: Require at least one updatable field besides `accountId`; currently a body with only `accountId` is accepted as a valid update.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client
participant API as API Route (PATCH)
participant VAL as validateUpdateAccountRequest
participant Auth as validateAuthContext
participant DB as Supabase Services
Client->>API: PATCH /api/accounts (JSON Body)
API->>VAL: NEW: validateUpdateAccountRequest(req)
VAL->>VAL: Parse JSON body
VAL->>VAL: CHANGED: Validate schema (Zod)
Note right of VAL: NEW: Validates "knowledges" as {name, url, type}[]
alt Invalid Schema
VAL-->>API: 400 Bad Request
API-->>Client: 400 Error Response
else Valid Schema
VAL->>Auth: NEW: Check auth and account permission
alt Auth Failure / No Permission
Auth-->>VAL: 401 Unauthorized (NextResponse)
VAL-->>API: 401 Unauthorized
API-->>Client: 401 Error Response
else Auth Success
Auth-->>VAL: AuthContext (accountId, orgId, token)
VAL->>VAL: NEW: Resolve accountId (body override check)
VAL-->>API: Validated UpdateAccountRequest
end
end
opt Request Validated & Authorized
API->>DB: updateAccountHandler(validatedPayload)
Note over DB: updateAccount, updateAccountInfo, etc.
DB-->>API: Updated data
API-->>Client: 200 OK (Account Data)
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
|
Tested the API preview directly: https://recoup-api-git-codex-migrate-account-acb222-recoupable-ad724970.vercel.app Verified with an authenticated bearer session that What I checked:
So the API preview is working end to end for authenticated account updates. |
sweetmantech
force-pushed
the
codex/migrate-account-update-api
branch
from
3576755 to
9f28804
Compare
| }); | ||
|
|
||
| export const updateAccountBodySchema = z.object({ | ||
| accountId: z.string().uuid("accountId must be a valid UUID"), |
There was a problem hiding this comment.
why is accountId required instead of optional?
There was a problem hiding this comment.
♻️ Duplicate comments (1)
lib/accounts/validateUpdateAccountRequest.ts (1)
13-15:⚠️ Potential issue | 🟠 MajorMake
accountIdoptional in the PATCH body and auth-derived by default.Line 14 currently makes
accountIdmandatory, which weakens the auth-first contract and forces redundant client input. Keep override optional and let auth context remain default source of truth (also update the PATCH JSDoc inapp/api/accounts/route.tsaccordingly).Suggested change
export const updateAccountBodySchema = z.object({ - accountId: z.string().uuid("accountId must be a valid UUID"), + accountId: z.string().uuid("accountId must be a valid UUID").optional(), name: z.string().optional(), instruction: z.string().optional(), organization: z.string().optional(), image: z.string().url("image must be a valid URL").optional().or(z.literal("")), jobTitle: z.string().optional(), roleType: z.string().optional(), companyName: z.string().optional(), knowledges: z.array(knowledgeSchema).optional(), });As per coding guidelines: "Never use
account_idin request bodies or tool schemas; always derive the account ID from authentication".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/accounts/validateUpdateAccountRequest.ts` around lines 13 - 15, The updateAccountBodySchema currently requires accountId, but per guidelines it should be optional and derived from auth context by default. Change accountId in updateAccountBodySchema to optional by using z.string().uuid().optional(), so clients do not need to provide it redundantly. Also ensure to update the related PATCH JSDoc in app/api/accounts/route.ts to reflect that accountId is optional and derived from authentication.
🧹 Nitpick comments (2)
lib/accounts/validateUpdateAccountRequest.ts (2)
1-1: Align validator filename with thevalidate<EndpointName>Body.tsconvention.This file is named
validateUpdateAccountRequest.ts; per validation-file convention, body schema validation should live invalidateUpdateAccountBody.ts(with schema + inferred type exported there).As per coding guidelines: "Create validate functions in
validate<EndpointName>Body.tsorvalidate<EndpointName>Query.tsfiles that export both the schema and inferred TypeScript type".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/accounts/validateUpdateAccountRequest.ts` at line 1, The validator file should follow the validate<EndpointName>Body.ts convention: rename validateUpdateAccountRequest.ts to validateUpdateAccountBody.ts, move or create an exported schema (e.g., updateAccountSchema) and an exported inferred TypeScript type (e.g., UpdateAccountBody) from that file, and update any imports that reference validateUpdateAccountRequest to import the schema and type from validateUpdateAccountBody.ts; if a validateUpdateAccountRequest function exists, keep or refactor it to use the exported updateAccountSchema/UpdateAccountBody from validateUpdateAccountBody.ts so schema and type are exported per the guidelines.
35-75: SplitvalidateUpdateAccountRequestinto smaller helpers.Line 35 through Line 75 mixes JSON parsing, schema error formatting, auth validation, and response shaping in one function (~41 lines). Extracting these into helpers will better match the small-focused-function guideline.
As per coding guidelines: "Flag functions longer than 20 lines" and "Keep functions small and focused".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/accounts/validateUpdateAccountRequest.ts` around lines 35 - 75, The validateUpdateAccountRequest function is doing parsing, schema-error formatting, auth validation and response shaping; split it into small helpers: create parseRequestBody(request) that wraps safeParseJson and returns the parsed body, create handleSchemaError(result) that formats and returns the NextResponse using updateAccountBodySchema error info and getCorsHeaders, create ensureAuthContext(request, accountId) that calls validateAuthContext and returns or forwards the NextResponse, and create buildValidatedResponse(authContext, resultData) that maps result.data fields into the ValidatedUpdateAccountRequest shape; then refactor validateUpdateAccountRequest to call these helpers in sequence (parseRequestBody -> schema check -> ensureAuthContext -> buildValidatedResponse) using the existing symbols validateUpdateAccountRequest, safeParseJson, updateAccountBodySchema, validateAuthContext, and getCorsHeaders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@lib/accounts/validateUpdateAccountRequest.ts`:
- Around line 13-15: The updateAccountBodySchema currently requires accountId,
but per guidelines it should be optional and derived from auth context by
default. Change accountId in updateAccountBodySchema to optional by using
z.string().uuid().optional(), so clients do not need to provide it redundantly.
Also ensure to update the related PATCH JSDoc in app/api/accounts/route.ts to
reflect that accountId is optional and derived from authentication.
---
Nitpick comments:
In `@lib/accounts/validateUpdateAccountRequest.ts`:
- Line 1: The validator file should follow the validate<EndpointName>Body.ts
convention: rename validateUpdateAccountRequest.ts to
validateUpdateAccountBody.ts, move or create an exported schema (e.g.,
updateAccountSchema) and an exported inferred TypeScript type (e.g.,
UpdateAccountBody) from that file, and update any imports that reference
validateUpdateAccountRequest to import the schema and type from
validateUpdateAccountBody.ts; if a validateUpdateAccountRequest function exists,
keep or refactor it to use the exported updateAccountSchema/UpdateAccountBody
from validateUpdateAccountBody.ts so schema and type are exported per the
guidelines.
- Around line 35-75: The validateUpdateAccountRequest function is doing parsing,
schema-error formatting, auth validation and response shaping; split it into
small helpers: create parseRequestBody(request) that wraps safeParseJson and
returns the parsed body, create handleSchemaError(result) that formats and
returns the NextResponse using updateAccountBodySchema error info and
getCorsHeaders, create ensureAuthContext(request, accountId) that calls
validateAuthContext and returns or forwards the NextResponse, and create
buildValidatedResponse(authContext, resultData) that maps result.data fields
into the ValidatedUpdateAccountRequest shape; then refactor
validateUpdateAccountRequest to call these helpers in sequence (parseRequestBody
-> schema check -> ensureAuthContext -> buildValidatedResponse) using the
existing symbols validateUpdateAccountRequest, safeParseJson,
updateAccountBodySchema, validateAuthContext, and getCorsHeaders.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 705020a2-aca9-4b85-a981-28937779b6c8
⛔ Files ignored due to path filters (1)
lib/accounts/__tests__/validateUpdateAccountRequest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (4)
app/api/accounts/route.tslib/accounts/updateAccountHandler.tslib/accounts/validateUpdateAccountBody.tslib/accounts/validateUpdateAccountRequest.ts
💤 Files with no reviewable changes (1)
- lib/accounts/validateUpdateAccountBody.ts
✅ Files skipped from review due to trivial changes (1)
- lib/accounts/updateAccountHandler.ts
- accountId is now derived from auth, optional as body override - Rejects requests with no updatable fields (refine validation) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Preview Deployment Testing ResultsPreview URL: PATCH /api/accounts
Fixes applied
|
2 participants
Summary
Verification
Summary by cubic
Secure PATCH
/api/accountswith auth-validated request handling, account access enforcement, and stricter payload rules.accountIdis now optional (derived from auth),knowledgesuse object-array schema, and empty updates are rejected.New Features
accountIdfrom auth when missing; provided values may be overridden.knowledges{ name, url, type }; reject empty updates with 400.Migration
accountId(derived from auth) and must include at least one updatable field.knowledgesas objects instead of strings.Written for commit 4a6dfc0. Summary will update on new commits.
Summary by CodeRabbit