fix: AI safety limits and env validation#713
Conversation
- Reduce nested agent stepCountIs from 100 to 20 and MAX_AGENT_DEPTH from 3 to 2 to prevent step explosion (100×100 = 10k tool executions) - Call validateEnv() in instrumentation.ts to catch env misconfig at startup - Add missing env vars to validation schema (OAUTH_STATE_SECRET, APPLE_SERVICE_ID, REALTIME_BROADCAST_SECRET, CRON_SECRET, COOKIE_DOMAIN) - Export env validation functions from @pagespace/lib/server - Add safeParseBody() utility for safe request.json() with Zod validation - Update drives and storage/check routes to return 400 on malformed JSON Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request introduces structured input validation across API routes using a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0084079de4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!result.success) { | ||
| return { | ||
| success: false, | ||
| response: Response.json({ error: result.error.issues }, { status: 400 }), |
There was a problem hiding this comment.
Returning result.error.issues here changes error from a string to an array of Zod issue objects for every schema failure (for example, POST /api/drives with a missing/empty name). Existing route contracts in this repo (e.g. src/app/api/drives/__tests__/route.test.ts) expect a string error payload, so this introduces a compatibility regression for clients/tests that render or compare error as text.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 96515a0. safeParseBody now flattens Zod issues into a semicolon-joined string, matching the { error: "string" } contract used across all existing routes. The drives schema also uses z.preprocess to normalize missing/empty name to the same "Missing name" error message.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/web/src/app/api/drives/route.ts (1)
62-64: Same Zod v4 error-option concern as storage/check schema.
z.string().min(1, 'Missing name')may need the{ error: ... }options object in v4; apply the same fix here if required.🛠️ Proposed fix
- name: z.string().min(1, 'Missing name'), + name: z.string().min(1, { error: 'Missing name' }),In Zod v4, does z.string().min accept a string message argument, or must it use the { error: ... } options object?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/api/drives/route.ts` around lines 62 - 64, createDriveSchema currently uses z.string().min(1, 'Missing name') which is incompatible with Zod v4; change the second argument to an options object like z.string().min(1, { message: 'Missing name' }) (same pattern used for the storage/check schema) so the error message is passed correctly to the zod validator.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/lib/validation/parse-body.ts`:
- Around line 25-30: safeParseBody currently returns result.error.issues (an
array) from the schema.safeParse call which diverges from the established
validation shape; change the error response to use
result.error.flatten().fieldErrors so the response.body follows the same {
field: [messages] } pattern used across routes, and update the corresponding
test expectation (the test asserting an array at the failing case) to expect an
object of fieldErrors instead; look for the safeParseBody function and its
Response.json({...}) return and replace result.error.issues with
result.error.flatten().fieldErrors, then adjust the test that asserts the error
shape to expect an object.
In `@packages/lib/src/config/env-validation.ts`:
- Around line 51-62: The optional secret/token schemas currently use
z.string().optional() which permits empty strings; update each to require at
least one character by changing OAUTH_STATE_SECRET, APPLE_SERVICE_ID,
STRIPE_SECRET_KEY, STRIPE_WEBHOOK_SECRET, REALTIME_BROADCAST_SECRET,
CRON_SECRET, and COOKIE_DOMAIN from z.string().optional() to
z.string().min(1).optional() so empty secrets are rejected (consistent with
DATABASE_URL and the existing superRefine checks for
CSRF_SECRET/ENCRYPTION_KEY).
---
Duplicate comments:
In `@apps/web/src/app/api/drives/route.ts`:
- Around line 62-64: createDriveSchema currently uses z.string().min(1, 'Missing
name') which is incompatible with Zod v4; change the second argument to an
options object like z.string().min(1, { message: 'Missing name' }) (same pattern
used for the storage/check schema) so the error message is passed correctly to
the zod validator.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web/src/app/api/drives/route.tsapps/web/src/app/api/storage/check/route.tsapps/web/src/instrumentation.tsapps/web/src/lib/ai/tools/__tests__/agent-communication-tools.test.tsapps/web/src/lib/ai/tools/agent-communication-tools.tsapps/web/src/lib/validation/__tests__/parse-body.test.tsapps/web/src/lib/validation/parse-body.tspackages/lib/src/config/env-validation.tspackages/lib/src/server.ts
- safeParseBody now returns flattened string error messages instead of
raw Zod issues array, matching existing { error: 'string' } API convention
- Drives route uses z.preprocess to produce consistent 'Missing name'
error for both missing field and empty string cases
- Hoisted Zod schemas to module level in drives and storage/check routes
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add .min(1) to all optional secret/token env var schemas to catch misconfiguration from empty values (e.g., CRON_SECRET= in .env). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
stepCountIsfrom 100 to 20 andMAX_AGENT_DEPTHfrom 3 to 2, preventing potential 10,000 tool execution chainsvalidateEnv()now runs ininstrumentation.tson server boot; added 5 missing env vars to schema (OAUTH_STATE_SECRET, APPLE_SERVICE_ID, REALTIME_BROADCAST_SECRET, CRON_SECRET, COOKIE_DOMAIN) with.min(1).optional()to reject empty stringssafeParseBody()utility returns 400 with string error message for malformed JSON instead of 500; applied to/api/drivesand/api/storage/checkroutesReview fixes
safeParseBodynow returns flattened string error messages (not Zod issues array), matching existing{ error: "string" }API conventionz.preprocessto produce consistent'Missing name'error for both missing field and empty string.min(1).optional()Test plan
parse-body.test.ts— 5 tests: valid JSON, malformed JSON, schema failure (string error), empty body, extra fieldsenv-validation.test.ts— 13 existing tests still passing with new schema fieldsagent-communication-tools.test.ts— depth limit test updated for new MAX_AGENT_DEPTH=2pnpm devstarts without env validation errors in development🤖 Generated with Claude Code