fix(management): reject null Claude toggle bodies - #1264
Conversation
📝 WalkthroughWalkthroughThe Claude integration PUT route now validates that the request body is a non-null object with a boolean ChangesClaude toggle validation
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
✅ Deterministic PR hygiene checks passed. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Ingwannu
left a comment
There was a problem hiding this comment.
Reviewed exact head c85d792d. The route now rejects null/non-object/array JSON before reading enabled, preserving the existing 400 contract instead of throwing. Focused coverage passed locally (11 tests), git diff --check passed, and typecheck passed. The substantive cross-platform shards, gates, privacy scan, keyring jobs, and platform tests are green; the macOS npm-global packaging job is still waiting for a runner, so do not merge until that final job completes successfully.
c85d792 to
a1cf0d9
Compare
Ingwannu
left a comment
There was a problem hiding this comment.
Re-reviewed rebased exact head a1cf0d9c. The PR diff is byte-equivalent in scope to the previously approved patch and still contains only the null-body guard plus its regression. Exact-head focused tests passed 11/11 and typecheck passed. Do not merge until the newly triggered exact-head CI is green.
a1cf0d9 to
a9dc822
Compare
⏳ DRAFT
What to do
Review readiness checklist
0/4 boxes ticked. This PR stays in draft until every box above is ticked. |
|
Rebased exact head Focused verification on the rebased state:
The branch is now exactly one commit ahead and zero behind |
Summary
nulland other non-object JSON bodies forPUT /api/native-integrations/claude;body.enabledand raising an unhandled TypeError;nullinput.Why
The route parses JSON into a value typed as an object and immediately reads
body.enabled. JSONnullparses successfully, so it bypasses the malformed-JSON catch and crashes at the property access. Ordinary non-boolean object values already receive a 400 response; top-level non-objects should follow the same contract.The guard now rejects null, arrays, and non-object values before reading
enabled. No valid toggle payload or persistence behavior changes.Verification
tests/native-claude-code-toggle.test.ts11/11 passed.b22e0e6d0): the same file 11/11 passed.bun x tsc --noEmit: passed.bun scripts/privacy-scan.ts: passed.git diff --check: passed.Checklist
Ready for review; upstream CI and automated review remain pending.
Summary by CodeRabbit
Bug Fixes
nullvalues, are now rejected with a clear HTTP 400 error instead of causing a server failure.Tests
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.