Skip to content

security/fix: comprehensive app hardening, restore OAuth login, and normalize note tags - #137

Open
jakharmonika364 wants to merge 10 commits into
Coder-s-OG-s:mainfrom
Ayush-Patel-56:security/fix-all-vulnerabilities
Open

security/fix: comprehensive app hardening, restore OAuth login, and normalize note tags#137
jakharmonika364 wants to merge 10 commits into
Coder-s-OG-s:mainfrom
Ayush-Patel-56:security/fix-all-vulnerabilities

Conversation

@jakharmonika364

Copy link
Copy Markdown
Collaborator

Description

This PR addresses all 19 security vulnerabilities identified in the workspace audit, fixes OAuth login, and resolves client-side rendering crashes caused by invalid data formats.


Key changes include:

  1. Credentials & Key Protection: Deleted the exposed client/JS/config.js containing API secrets, added it to .gitignore, and built a server-side AI proxy at /api/ai/generate to keep the Groq API key hidden.

  2. Backend Hardening:

    • Implemented strict whitelisting to prevent mass assignment exploits during note creation and updates.
    • Removed detailed internal error stack traces from API responses.
    • Introduced rate-limiting on all routes with stricter limits on authentication endpoints.
    • Configured CORS boundaries to allow only valid dashboard origins.
    • Added custom origin-checking CSRF protection middleware for all state-changing endpoints (POST/PUT/DELETE).
    • Added request input validators (via express-validator) for write routes.
    • Sanitized user data returned on the /api/auth/user endpoint to strip database identifiers and provider-specific details.
  3. Frontend XSS Protection: Integrated DOMPurify into HTML rendering and AI-generated outputs, utilizing a validated SRI hash in app.html to guarantee script integrity.

  4. Login & Middleware Fixes: Restored OAuth functionality by switching session cookies to SameSite: lax (allowing redirects) and fixing the execution order of body-parsers, CORS, and authentication middleware.

  5. Data Resiliency: Normalized the note tags field to always fall back to a clean array of strings, resolving client-side note.tags.map is not a function console errors when reading legacy databases.


Type of Change

  • Bug fix
  • New feature
  • Enhancement
  • Documentation update

Checklist

  • Code follows project structure
  • Tested locally
  • No unnecessary files added

… bundle

- Added JS/config.js to .gitignore (root-level file was not protected)
- Removed GROQ_API_KEY from the config object written to client/JS/config.js
  by server.js on startup — the key must never reach the browser
- API key will only be consumed server-side via the upcoming AI proxy route (C1, C2, C4)
- Created server/routes/ai.js: authenticated POST /api/ai/generate endpoint
  that reads GROQ_API_KEY from process.env and proxies to Groq API
- Mounted /api/ai route in server.js
- Rewrote client/JS/geminiAPI.js to call /api/ai/generate instead of
  contacting Groq directly — the API key never reaches the browser
- Proxy enforces: authentication check, prompt validation (non-empty, max 10k chars),
  graceful error handling without leaking internals
…okie

Two bugs introduced by the security hardening commit:

1. SameSite: 'strict' broke OAuth login.
   When Google/GitHub redirects back to /api/auth/.../callback the browser
   treats it as a cross-site navigation and refuses to send a SameSite:strict
   cookie. Passport then cannot find the stored OAuth state in the session,
   causing the callback to fail silently.
   Fix: reverted to SameSite: 'lax' which allows OAuth GET redirects while
   still blocking cross-site POST/PUT/DELETE requests (CSRF for state changes).

2. Middleware registered out of order.
   Rate limiters and CSRF middleware were registered before express.json() and
   the session middleware, meaning they could not read request bodies or session
   data. Reordered to: body parsers -> CORS -> session/passport -> rate limit
   -> CSRF -> routes.
@vercel

vercel Bot commented Jun 6, 2026

Copy link
Copy Markdown

@jakharmonika364 is attempting to deploy a commit to the kunal628-hue's projects Team on Vercel.

A member of the Team first needs to authorize it.

- Remove import config from './config.js' from studentHub.js and
  codeWorkspace.js; config.js was deleted in the security PR since the
  GROQ_API_KEY moved to the server .env
- Update checkAPIKey() in both files to probe the server-side
  /api/ai/generate proxy instead of reading a non-existent client key;
  401/429 responses are treated as 'AI available' (just needs login)
- Fix geminiAPI.js to throw real Error objects on all failure paths
  (401, non-ok, empty response) instead of returning '[AI Assistant]: ...'
  strings that silently bypassed JSON parsing and caused the
  'Unable to parse AI response as JSON' toast error
- Remove the now-dead aiResponse.includes('Error:') string checks in
  studentHub.js (all three call sites) since errors are thrown properly

@Ayush-Patel-56 Ayush-Patel-56 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Good to me. @Kunal317-singhi I tried it locally and every features is working. Also I can't found any lag issue in the app.

You also tried it locally once or approve the vercel deployment for better results.

From my end, we are good to go with this PR, as It solves mostly all the vulnurabilities and bugs.

Thanks!

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.

2 participants