[Bug]: return 400 for invalid JSON in profile upload#4362
Conversation
|
@SueJianjian is attempting to deploy a commit to the Anurag Mishra's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
CodeAnt AI is reviewing your PR. |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds ChangesInput Controller JSON Validation Fix
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant inputupload
participant parseInputDataPayload
Client->>inputupload: POST req.body.data
inputupload->>parseInputDataPayload: parseInputDataPayload(data)
alt invalid JSON
parseInputDataPayload-->>inputupload: null
inputupload-->>Client: 400 Invalid JSON format in request body
else valid payload
parseInputDataPayload-->>inputupload: normalized data
inputupload-->>Client: continue processing
end
Related issues: Suggested labels: bug, backend Suggested reviewers: none 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
codeant-ai
Bot
added
the
size:L
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| if (!data) { | ||
| return {}; |
There was a problem hiding this comment.
Suggestion: The falsy check treats an empty string payload as valid and returns an empty object, so malformed JSON like "" bypasses the new 400 path and continues into database/workflow logic. Only treat undefined/null as “no payload” and let string inputs (including empty strings) go through JSON parsing so malformed values correctly return 400. [falsy zero check]
Severity Level: Major ⚠️
- ⚠️ Empty-string payloads bypass 400 and hit database.
- ⚠️ Profiles created with missing fields from empty payloads.
- ⚠️ Invalid client input not surfaced as client error.
- ⚠️ Tests omit coverage for empty-string payload case.Steps of Reproduction ✅
1. In backend/src/controllers/input.controller.test.js (lines 24-63), create a test
similar to "returns 400 for invalid JSON payloads before touching persistence" but set
req.body.data to an empty string: { body: { data: "" }, user: { id: "user-id" }, file:
null } and call inputupload(req, res).
2. The test calls inputupload in backend/src/controllers/input.controller.js (line 23),
which immediately computes parsedData = parseInputDataPayload(req.body.data) at line 25.
3. parseInputDataPayload in backend/src/controllers/input.controller.js (lines 7-21)
receives data === "" and executes the falsy check at lines 8-9 ("if (!data) { return {};
}"), returning an empty object instead of attempting JSON.parse and returning null for
invalid JSON.
4. Back in inputupload (lines 27-31), the guard "if (parsedData === null)" does not
trigger because parsedData is {}, so the 400 response is skipped; execution continues into
userModel.findById(req.user.id) at line 33 and subsequent resume-processing and database
create logic at lines 87-96, meaning an effectively invalid/empty payload bypasses the
intended early 400 path and is treated as a normal profile upload.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** backend/src/controllers/input.controller.js
**Line:** 8:9
**Comment:**
*Falsy Zero Check: The falsy check treats an empty string payload as valid and returns an empty object, so malformed JSON like `""` bypasses the new 400 path and continues into database/workflow logic. Only treat `undefined`/`null` as “no payload” and let string inputs (including empty strings) go through JSON parsing so malformed values correctly return 400.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished reviewing your PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/security-scan.yml:
- Around line 178-180: The security summary is using only job result values for
Snyk and OSV Scanner, which can report Passed even when vulnerabilities were
found or the scan was skipped. Update the summary logic in the security scan
workflow to base the status on the scanners’ actual outcome/outputs rather than
`needs.snyk.result` and `needs.osv-scan.result`, and apply the same change to
the PR comment block. Use the existing Snyk and OSV Scanner job identifiers and
their emitted scan status fields to distinguish passed, skipped, and issues
found.
- Around line 75-77: The Checkout repository step in the security scan workflow
is persisting GitHub credentials by default, which can expose the token to npm
install scripts or the Snyk action. Update the actions/checkout@v4 usage in the
checkout step to disable credential persistence by setting persist-credentials
to false while keeping the existing SNYK_TOKEN guard intact.
- Line 125: The conditional in the workflow job uses the wrong fork check, since
github.event.repository.fork reflects the base repo instead of the PR head repo.
Update the condition in the security scan workflow to use
github.event.pull_request.head.repo.fork == false alongside the pull_request
event check, so the job is skipped for PRs coming from forked head repositories.
Use the existing if expression in the workflow as the location to adjust.
- Around line 92-94: The Snyk GitHub Actions step in the security scan workflow
is using a mutable branch reference, so update the uses value for the Run Snyk
to check for vulnerabilities step in security-scan.yml from
snyk/actions/node@master to a pinned, vetted full commit SHA. Keep the existing
step name and SNYK_TOKEN gate unchanged, and only replace the action reference
so the workflow remains stable on protected-branch runs.
In `@backend/src/controllers/input.controller.js`:
- Around line 7-21: The parseInputDataPayload helper currently treats empty
strings as missing data and also accepts any parsed JSON value, including arrays
and primitives, which lets invalid payloads bypass the controller’s 400
handling. Update parseInputDataPayload in input.controller.js to return {} only
for null/undefined input, reject empty strings as invalid, and only return a
value when the payload parses to a plain object; otherwise return null so the
controller logic that consumes parsedData can continue to fail fast on
non-object input.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c490b5e5-e0ca-4fb8-ad28-6e18be7cd377
📒 Files selected for processing (5)
.github/workflows/security-scan.ymlbackend/src/controllers/input.controller.jsbackend/src/controllers/input.controller.test.jsbackend/src/routes/portfolio.jsfrontend/src/components/portfolio/templates/Fake_WebOS_Operating_System/index.jsx
💤 Files with no reviewable changes (1)
- backend/src/routes/portfolio.js
| - name: Checkout repository | ||
| if: env.SNYK_TOKEN != '' | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files .github/workflows/security-scan.yml
wc -l .github/workflows/security-scan.yml
sed -n '1,220p' .github/workflows/security-scan.ymlRepository: anurag3407/career-pilot
Length of output: 7355
Disable credential persistence on the Snyk checkout. npm install runs before the Snyk action, so the default GitHub token in .git/config can be read by dependency scripts or the third-party action. Set persist-credentials: false here.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 75-77: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/security-scan.yml around lines 75 - 77, The Checkout
repository step in the security scan workflow is persisting GitHub credentials
by default, which can expose the token to npm install scripts or the Snyk
action. Update the actions/checkout@v4 usage in the checkout step to disable
credential persistence by setting persist-credentials to false while keeping the
existing SNYK_TOKEN guard intact.
Source: Linters/SAST tools
| - name: Run Snyk to check for vulnerabilities | ||
| working-directory: ./backend | ||
| if: env.SNYK_TOKEN != '' | ||
| uses: snyk/actions/node@master |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== security-scan.yml excerpt =="
sed -n '70,110p' .github/workflows/security-scan.yml
echo
echo "== action reference search =="
rg -n "uses:\s+.*`@master`|uses:\s+.*@[A-Za-z0-9._/-]+$|uses:\s+.*@[0-9a-f]{7,}" .github/workflows -g '*.yml' -g '*.yaml' || true
echo
echo "== any snyk references =="
rg -n "snyk/actions|SNYK_TOKEN|Run Snyk" . -g '*.yml' -g '*.yaml' -g '*.md' || trueRepository: anurag3407/career-pilot
Length of output: 3963
Pin snyk/actions/node to a commit SHA. The step runs with SNYK_TOKEN from secrets, so @master can change behavior on protected-branch runs. Use a vetted full commit SHA instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/security-scan.yml around lines 92 - 94, The Snyk GitHub
Actions step in the security scan workflow is using a mutable branch reference,
so update the uses value for the Run Snyk to check for vulnerabilities step in
security-scan.yml from snyk/actions/node@master to a pinned, vetted full commit
SHA. Keep the existing step name and SNYK_TOKEN gate unchanged, and only replace
the action reference so the workflow remains stable on protected-branch runs.
| name: GitHub Dependency Review | ||
| runs-on: ubuntu-latest | ||
| if: github.event_name == 'pull_request' | ||
| if: github.event_name == 'pull_request' && !github.event.repository.fork |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the workflow context around the reported line.
file=".github/workflows/security-scan.yml"
wc -l "$file"
sed -n '110,140p' "$file"Repository: anurag3407/career-pilot
Length of output: 1195
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for similar fork checks in workflows and docs in the repo.
rg -n "repository\.fork|head\.repo\.fork|pull_request" .github/workflows . -g '*.yml' -g '*.yaml' -g '*.md'Repository: anurag3407/career-pilot
Length of output: 597
Use the PR head repo fork flag here
github.event.repository.fork only reflects whether the base repository is itself a fork, so PRs opened from forked head repos still satisfy this check. Switch to github.event.pull_request.head.repo.fork == false.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/security-scan.yml at line 125, The conditional in the
workflow job uses the wrong fork check, since github.event.repository.fork
reflects the base repo instead of the PR head repo. Update the condition in the
security scan workflow to use github.event.pull_request.head.repo.fork == false
alongside the pull_request event check, so the job is skipped for PRs coming
from forked head repositories. Use the existing if expression in the workflow as
the location to adjust.
| echo "| npm audit | ${{ needs.npm-audit.result == 'success' && 'Passed' || 'Failed' }} |" >> "$GITHUB_STEP_SUMMARY" | ||
| echo "| Snyk | ${{ needs.snyk.result == 'success' && 'Passed' || needs.snyk.result == 'skipped' && 'Skipped' || 'Issues found' }} |" >> "$GITHUB_STEP_SUMMARY" | ||
| echo "| OSV Scanner | ${{ needs.osv-scan.result == 'success' && 'Passed' || 'Issues found' }} |" >> "$GITHUB_STEP_SUMMARY" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect relevant workflow sections.
grep -n -A6 -B8 'npm audit\|needs\.snyk\.result\|needs\.osv-scan\.result\|continue-on-error\|snyk\|osv-scan' .github/workflows/security-scan.yml
# Show the whole file with line numbers if manageable.
wc -l .github/workflows/security-scan.ymlRepository: anurag3407/career-pilot
Length of output: 7010
Treat scan outcomes, not job success, in the summary. needs.snyk.result and needs.osv-scan.result can be success even when the scanner found vulnerabilities because both jobs use continue-on-error; Snyk also lands on success when the token is missing, so this will report Passed for skipped or failed scans. Also applies to the PR comment block.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/security-scan.yml around lines 178 - 180, The security
summary is using only job result values for Snyk and OSV Scanner, which can
report Passed even when vulnerabilities were found or the scan was skipped.
Update the summary logic in the security scan workflow to base the status on the
scanners’ actual outcome/outputs rather than `needs.snyk.result` and
`needs.osv-scan.result`, and apply the same change to the PR comment block. Use
the existing Snyk and OSV Scanner job identifiers and their emitted scan status
fields to distinguish passed, skipped, and issues found.
|
CodeAnt AI is running Incremental review |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:L
|
CodeAnt AI Incremental review completed. |
|
CodeAnt AI is running Incremental review |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:L
|
CodeAnt AI Incremental review completed. |
1 participant
User description
Summary
eq.body.data contains malformed JSON in the profile upload controller
Issue
Closes #4261
Validation
ode --test src/controllers/input.controller.test.js
px eslint src/controllers/input.controller.js src/controllers/input.controller.test.js
Notes
Summary by CodeRabbit
Bug Fixes
data, rejecting invalid or empty JSON with a clear HTTP 400 error message.Tests
CodeAnt-AI Description
Reject invalid profile upload JSON with a 400 response
What Changed
Impact
✅ Clearer profile upload errors✅ Fewer failed uploads from bad JSON✅ Less wasted processing on invalid requests💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.