Skip to content

Fix critical security vulnerabilities#71

Open
beta-devin-ai-integration[bot] wants to merge 1 commit into
masterfrom
devin/1775583824-fix-security-issues
Open

Fix critical security vulnerabilities#71
beta-devin-ai-integration[bot] wants to merge 1 commit into
masterfrom
devin/1775583824-fix-security-issues

Conversation

@beta-devin-ai-integration

Copy link
Copy Markdown

Summary

Security audit of the backend (bog-api/) identified and fixes 10 issues:

  • Hardcoded secrets in example.env: Replaced real Stripe test keys (sk_test_..., whsec_...) and a weak JWT secret with placeholder values
  • Plaintext password storage: Admin password change used User.updateOne() which bypasses Mongoose schema setters — the bcrypt hash was never applied. Changed to findOne() + .save() to trigger the setter
  • Insecure auth cookie: Added secure (production) and sameSite: "strict" flags
  • IDOR on video re-render: Any authenticated user could re-render any other user's video. Added owner: req.user._id filter
  • NoSQL injection: username/email from req.body were passed directly into MongoDB queries. Added typeof === "string" validation on registration and admin user creation
  • Permissive CORS default: Changed check from !== "production" to === "development" so CORS isn't accidentally open when NODE_ENV is unset
  • Stripe webhook info leak: Removed err.message from response and console.log(session) of payment data
  • dotenv load order: Moved dotenv.config() before modules that read process.env

Review & Testing Checklist for Human

  • Rotate Stripe test keys — the old sk_test_* and whsec_* values are still in git history. They should be rotated in the Stripe dashboard immediately
  • Verify sameSite: "strict" doesn't break Stripe checkout redirect — after completing payment on Stripe's hosted checkout page, the redirect back to the app relies on the auth cookie being sent. strict blocks cookies on cross-site navigations; if this breaks the flow, change to "lax"
  • Verify CORS change doesn't break staging/CI environments — if any non-production deployment uses a NODE_ENV value other than "development" (e.g., "staging", "test", or unset), CORS will now be disabled. Check that all environments that need CORS set NODE_ENV=development
  • Test admin password change — confirm that passwords set via PUT /api/admin/users/:id/change-password are properly bcrypt-hashed in the database (not stored as plaintext)
  • Test video re-render — confirm a user cannot re-render another user's video by passing a foreign video ID in body.rerenderVideo

Notes

  • The NoSQL injection fix only covers username/email on registration and admin user creation. Other endpoints (e.g., /api/auth login) also pass req.body fields into queries without type checks — these should be addressed in a follow-up
  • The /videos static file route still serves any file to any authenticated user without ownership checks — flagged in the audit but not fixed here as it requires an architectural change
  • Tests could not be run locally (Jest hangs, likely needs a running MongoDB instance)

Link to Devin session: https://app.beta.devin.ai/sessions/01fe4df39ecd47ed8a6c83cb255aa88a
Requested by: @FOLLGAD

- Replace hardcoded Stripe keys and weak JWT secret in example.env with placeholders
- Fix admin password change to use findOne+save (triggers bcrypt setter) instead of updateOne (stores plaintext)
- Add secure and sameSite flags to auth cookie
- Fix IDOR on video re-render by adding owner check
- Add NoSQL injection protection with string type validation on registration and admin user creation
- Tighten CORS to only allow permissive mode when NODE_ENV=development (not just != production)
- Sanitize Stripe webhook error response to not leak internal details
- Remove console.log of sensitive Stripe session data
- Move dotenv.config() before modules that depend on env vars

Co-Authored-By: ahlback.emil+coggitgrant <me@emil.zip>
@beta-devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant