🔒 [security fix] Fix Mass Assignment in Job and Blog Update#6
🔒 [security fix] Fix Mass Assignment in Job and Blog Update#6Adityavanjre wants to merge 2 commits into
Conversation
- Replace Object.assign with explicit field mapping in updateJob and updateBlog controllers. - Prevents malicious users from overwriting unauthorized fields like createdAt or status by sending them in the request body. - Whitelists only the intended fields for updates as per the model schema. Co-authored-by: Adityavanjre <66417028+Adityavanjre@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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: Free Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR modifies two controller update methods— ChangesSelective Field Updates in Controllers
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. 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: 99d0a065fd
ℹ️ 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 (blog) { | ||
| Object.assign(blog, req.body); | ||
| const { title, author, category, image, excerpt, content, readTime, tags, slug, status } = req.body; |
There was a problem hiding this comment.
When an admin/API client sends date in PUT /api/blogs/:id, this new whitelist silently drops it, even though date is a first-class Blog schema field (agency/server/models/Blog.js) and is rendered in the blog list/detail UI. The old Object.assign path allowed correcting or backdating the displayed publish date; after this change those requests still return success but leave the public date stale, so include date in the allowed fields unless it is intentionally being made immutable.
Useful? React with 👍 / 👎.
- Replace Object.assign with explicit field mapping in Job and Blog controllers to prevent mass assignment vulnerabilities. - Update root package.json ajv override from ^6.12.6 to ^8.18.0 to resolve CI build error in nexus/backend. Co-authored-by: Adityavanjre <66417028+Adityavanjre@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77476a8d8e
ℹ️ 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".
| "@nestjs/common": "^11.1.18", | ||
| "@nestjs/core": "^11.1.18", | ||
| "ajv": "^6.12.6" | ||
| "ajv": "^8.18.0" |
There was a problem hiding this comment.
This root override applies to Ajv consumers across the workspace, but the committed lockfile still contains v6-only consumers such as @develar/schema-utils requiring ajv: ^6.12.0 and ajv-keywords@3.5.2 peering on ajv: ^6.9.1. I checked the local npm ci man page, which states that if lockfile dependencies do not match package.json, npm ci exits instead of updating the lock; root CI/install jobs will either fail as out-of-sync or, after regenerating, force Ajv 8 into packages used by electron-builder/fork-ts-checker that expect Ajv 6.
Useful? React with 👍 / 👎.
1 participant
🎯 What:
Fixed a mass assignment vulnerability in the Job and Blog update controllers.
The previous use of
Object.assign(document, req.body)allowed clients to modify any property on the Mongoose document, including those not intended for client-side updates (e.g., system-generated fields likecreatedAt, or restricted fields). This could lead to data integrity issues or privilege escalation if sensitive fields were present in the model.🛡️ Solution:
Replaced the direct merging of
req.bodywith an explicit field whitelist. Only fields specified in the model schema for these entities are now updated, and only if they are present in the request body.Affected Files:
agency/server/controllers/jobController.js: Whitelistedrole,type,location,description,requirements, andisActive.agency/server/controllers/blogController.js: Whitelistedtitle,author,category,image,excerpt,content,readTime,tags,slug, andstatus.Verification was performed using a reproduction script that attempted to overwrite
createdAtand add an unauthorizedisAdminfield, confirming that the fix correctly ignores these fields while still allowing valid updates.PR created automatically by Jules for task 2451609093056489458 started by @Adityavanjre
Summary by CodeRabbit