Skip to content

Conversation

@Itsnotaka
Copy link
Contributor

What does this PR do?

CleanShot.2025-11-12.at.18.47.59.mp4
CleanShot.2025-11-12.at.18.48.38.mp4

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

@Itsnotaka Itsnotaka requested a review from Marfuen November 12, 2025 14:00
@CLAassistant
Copy link

CLAassistant commented Nov 12, 2025

CLA assistant check
All committers have signed the CLA.

@vercel
Copy link

vercel bot commented Nov 12, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
app Ready Ready Preview Comment Nov 14, 2025 0:04am
portal Ready Ready Preview Comment Nov 14, 2025 0:04am

@comp-ai-code-review
Copy link

comp-ai-code-review bot commented Nov 12, 2025

🔒 Comp AI - Security Review

🟡 Risk Level: MEDIUM

One low-severity OSV: GHSA-rwvc-j5jr-mgvh in npm package 'ai' v5.0.0 — filetype whitelist bypass on uploads; fixed in 5.0.52.


📦 Dependency Vulnerabilities

🟢 NPM Packages (LOW)

Risk Score: 2/10 | Summary: 1 low CVE found

Package Version CVE Severity CVSS Summary Fixed In
ai 5.0.0 GHSA-rwvc-j5jr-mgvh LOW N/A Vercel’s AI SDK's filetype whitelists can be bypassed when uploading files 5.0.52

🛡️ Code Security Analysis

View 4 file(s) with issues

🟡 apps/app/src/app/(app)/[orgId]/layout.tsx (MEDIUM Risk)

# Issue Risk Level
1 User-controlled orgId used directly in DB queries and redirects MEDIUM
2 publicAccessToken cookie accepted and passed on without validation MEDIUM

Recommendations:

  1. Validate and constrain orgId before use: enforce a strict format (e.g., UUID regex) or cast to the expected type and reject invalid values before calling the ORM or performing redirects.
  2. When building URLs with user-controlled segments, encode path segments (e.g., encodeURIComponent) or use framework routing helpers to avoid open redirect / path injection issues.
  3. Ensure ORM queries are executed with parameterized queries (most ORMs like Prisma do this by default). Nevertheless, validate input shape and type to avoid logic bugs or unexpected behavior.
  4. Do server-side verification of publicAccessToken before trusting it in server-rendered components: check signature/expiry/permissions against a secure token store or auth service rather than blindly passing the cookie value into providers.
  5. Set sensitive cookies (like tokens) with Secure, SameSite and HttpOnly attributes to reduce theft and tampering from client-side scripts. Consider storing tokens server-side if possible.
  6. Add explicit authorization checks on every operation that uses an orgId (defense in depth): after loading organization, verify the session user has the required roles/permissions for the requested action.
  7. Centralize and reuse validation and authorization middleware for route params so all handlers enforce the same checks (e.g., a middleware that validates orgId and guarantees an Organization object is loaded).

🟢 apps/app/src/components/layout/app-shell.tsx (LOW Risk)

# Issue Risk Level
1 Cookie set client-side without Secure/SameSite/HttpOnly flags LOW
2 UI state stored in localStorage accessible to JS (XSS risk) LOW
3 Client-set cookie will be sent with requests and may expose state LOW

Recommendations:

  1. Prefer setting this preference cookie from the server so it can include HttpOnly and Secure flags. HttpOnly cannot be set from client-side JS.
  2. If the cookie must be set client-side, at minimum add ;Secure and ;SameSite=Lax (or Strict) and restrict the domain/path. Consider using the __Host- or __Secure- prefix when applicable.
  3. Avoid placing any sensitive data in localStorage. Only store non-sensitive UI state. If data is sensitive, store it in server-side session storage or an HttpOnly cookie.
  4. Harden against XSS to prevent attackers from reading/modifying localStorage or document.cookie: use input/output encoding, strict Content Security Policy (CSP), and other XSS mitigations.
  5. Limit cookie scope (path, domain) and lifetime, and consider encoding/validating values to make tampering less useful.

🟡 apps/app/src/components/main-menu.tsx (MEDIUM Risk)

# Issue Risk Level
1 Protected flag not enforced in UI; protected items may be shown or linked MEDIUM
2 organizationId interpolated into hrefs without validation/encoding MEDIUM
3 Client-side access control can be bypassed if server lacks checks MEDIUM

Recommendations:

  1. Enforce authorization server-side for all protected routes and APIs. The UI must never be the sole enforcement point.
  2. Use authenticated/authorized checks to hide or disable protected menu items on the client (e.g., evaluate user roles/permissions and filter items where item.protected === true). Do not assume presence of the protected flag itself implies enforcement anywhere.
  3. Validate organizationId against an expected format (e.g., UUID or numeric ID) on both client and server. Reject or canonicalize unexpected values.
  4. Encode organizationId when interpolating into URLs: e.g., const safeOrg = encodeURIComponent(organizationId || ''); use safeOrg when building link hrefs to avoid malformed URLs or path injection.
  5. Sanitize and validate any data coming from untrusted sources before using it to construct URLs or renderable content. Prefer route helpers or Next.js dynamic route APIs that accept params instead of raw string concatenation.
  6. Audit server endpoints backing these routes to ensure they verify the requesting user's access to the requested organization and resource; add tests to assert unauthorized requests are rejected.

🟡 apps/app/src/components/organization-switcher.tsx (MEDIUM Risk)

# Issue Risk Level
1 State-changing action lacks explicit CSRF protection MEDIUM
2 Organization IDs exposed in DOM/URL (info disclosure) MEDIUM
3 CommandItem value concatenates id and name, may leak data MEDIUM

Recommendations:

  1. Ensure server-side protection for changeOrganizationAction: validate origin/CSRF token or use SameSite cookies and verify user session/intent on the server. Do not rely solely on client-side checks.
  2. On the server that handles changeOrganizationAction, perform strict authorization checks (ACLs) to ensure the calling user may switch to the requested organization ID.
  3. Avoid exposing raw internal identifiers where possible. Consider using opaque/surrogate IDs (e.g., UUIDv4 tokens or hashed IDs) for public URLs or ensure every request enforces authorization checks server-side so IDs themselves are not sufficient to access resources.
  4. Avoid embedding sensitive information into DOM attributes that might be scraped. If you must include IDs for UI interactions, scope them (e.g., use short non-guessable tokens) and ensure any server endpoints validate the requesting user's rights to that ID.
  5. Validate and canonicalize client-sourced preference data (like sortOrder from localStorage) before using it, even if only for display — restrict to an allow-list of known values ('alphabetical','recent') to avoid unexpected behavior.

💡 Recommendations

View 3 recommendation(s)
  1. Upgrade the ai dependency to >= 5.0.52 (update package.json / lockfile and rebuild) so the GHSA-rwvc-j5jr-mgvh fix is applied.
  2. Harden server-side upload handling: validate file contents (magic bytes / MIME sniffing) and enforce an allow-list of permitted types before persisting or processing uploads.
  3. Add automated tests for upload handling that assert disallowed filetypes are rejected even if filename/extension or declared MIME type are spoofed.

Powered by Comp AI - AI that handles compliance for you. Reviewed Nov 14, 2025

@Itsnotaka
Copy link
Contributor Author

This will be pending until we have a clearer sight on design system

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.

4 participants