Skip to content

Conversation

@github-actions
Copy link
Contributor

This is an automated pull request to release the candidate branch into production, which will trigger a deployment.
It was created by the [Production PR] action.

* feat: add contractor role

* feat: update role checks to include contractor in various actions
@comp-ai-code-review
Copy link

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

🔒 Comp AI - Security Review

🔴 Risk Level: HIGH

1 OSV CVE: GHSA-rwvc-j5jr-mgvh (ai v5.0.0). Prisma schema persists JWKS privateKey and Account.password as plain strings. Code paths expose stored-XSS (comments) and CSV formula injection risks.


📦 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 8 file(s) with issues

🟡 apps/app/src/actions/organization/remove-employee.ts (MEDIUM Risk)

# Issue Risk Level
1 Detailed internal error messages returned to client MEDIUM
2 memberId not validated as UUID or sanitized MEDIUM
3 Role check uses exact match on currentUserMember.role, may fail for multi-role strings MEDIUM
4 No guard ensuring at least one owner remains MEDIUM
5 TOCTOU: permission checks done before DB mutation allow race conditions MEDIUM

Recommendations:

  1. Validate memberId strictly as a UUID in the zod schema (e.g., z.string().uuid()) and/or assert its format before using it in DB calls.
  2. Do not return raw error.message to clients. Return a generic client-facing error (e.g., 'Failed to remove employee') and log the full error server-side with context and a correlation id.
  3. Normalize role handling consistently for both the current user and target user. Split role strings by comma, trim whitespace, and check includes(...) rather than exact equality. Consider storing roles as an array in the DB to avoid parsing issues.
  4. Enforce the invariant that at least one owner remains: before removing/updating an owner-role, count owners in the organization (e.g., SELECT COUNT(*) WHERE role includes 'owner'). Block the operation if it would remove the last owner or require promoting another member first.
  5. Move authorization checks and the destructive mutations into a single database transaction or use row-level locks (SELECT ... FOR UPDATE) so you re-validate roles/ownership inside the transaction to avoid TOCTOU race conditions. Re-check the target's roles/ownership status inside the transaction before delete/update.
  6. When removing a role from a comma-separated string, handle edge cases (leading/trailing commas, resulting empty string). If no roles remain, decide whether to delete the member or set role to NULL/empty in a well-defined way.
  7. Add server-side logging and monitoring of these privileged actions (who performed them, timestamps, affected ids) to detect abuse and aid incident response.

🔴 apps/app/src/actions/policies/accept-requested-policy-changes.ts (HIGH Risk)

# Issue Risk Level
1 Approver spoofing: approverId not verified against current user HIGH
2 Missing validation: approver's org membership or role not checked HIGH
3 Stored comment not sanitized (stored XSS risk) HIGH
4 Logging raw errors may leak sensitive data in server logs HIGH

Recommendations:

  1. Require the approverId to match the authenticated actor (e.g., resolve the current user's member id and ensure it equals policy.approverId) instead of trusting an approverId supplied by the client.
  2. Verify the approver is an active member of the same organization and has the appropriate approver role/permission before allowing the action (query the member table for userId → memberId and check role/permissions).
  3. Sanitize, validate, or HTML-escape comment content before persisting (or persist raw but ensure all render paths escape it). Consider whitelist sanitization or using a library like DOMPurify on render if HTML is allowed, or strip/encode HTML before store.
  4. Avoid logging raw error objects. Log minimal contextual information and, if needed, redact or sanitize sensitive fields before logging. Use structured logging with severity and a correlation id rather than dumping whole exceptions to console in production.
  5. Remove or repurpose unused input fields (the schema includes entityId but the code uses id) or validate they match to avoid confusion/abuse.

🟡 apps/app/src/app/(app)/[orgId]/people/[employeeId]/page.tsx (MEDIUM Risk)

# Issue Risk Level
1 Inconsistent orgId vs session.activeOrganizationId usage MEDIUM
2 Missing authorization check to ensure user may view employee data MEDIUM
3 URL params (employeeId/orgId) used without validation before DB/API queries MEDIUM
4 Sensitive data logged (member emails, error details) to console MEDIUM
5 Unvalidated fleet label ID used in external API path MEDIUM

Recommendations:

  1. Use a single authoritative orgId for all server-side checks: derive organizationId from the authenticated session (session.session.activeOrganizationId) and verify any orgId URL param matches it. Do not use the URL orgId for permission checks.
  2. Compute currentUserMember using the session's activeOrganizationId (not the URL orgId). If the user is not a member of the organization, return 403 or redirect — do not proceed rendering data for that org.
  3. Enforce authorization on every data access path: verify the current user has permission to view/edit the target employee before rendering any edit controls (e.g., use server-side checks that compare the session user membership/role for the same organizationId used to fetch the employee).
  4. Validate and sanitize route params before use: check employeeId/orgId formats (UUIDs or expected patterns) with a validation library (zod, Joi) or explicit regex and fail fast with 400 on invalid input.
  5. Avoid logging PII or raw error objects in production. Redact or hash user emails and log only non-sensitive error identifiers. Use structured logging with environments controlling verbosity.
  6. Validate and normalize fleetDmLabelId before including it in external API paths. Prefer whitelisting/format checks (UUIDs/numeric id), and URL-encode path segments. Handle unexpected values by skipping the fleet call and logging a non-PII warning.
  7. Although Prisma/ORM parameter binding prevents classic SQL injection, avoid trusting unchecked route params for sensitive actions; always validate input shapes and types.
  8. Add explicit error handling and user-facing responses for authorization failures (403) versus not found (404) to avoid leaking resource existence semantic differences where appropriate.

🟡 apps/app/src/app/(app)/[orgId]/people/all/components/InviteMembersModal.tsx (MEDIUM Risk)

# Issue Risk Level
1 Validation only on client; server-side validation not enforced MEDIUM
2 Console logs leak user emails and form validation errors MEDIUM
3 Raw server error messages shown to users MEDIUM
4 Weak CSV type check (MIME/extension only) MEDIUM
5 File size/type checks only client-side MEDIUM
6 Potential CSV formula injection (spreadsheet formulas) MEDIUM
7 Client-supplied roles used without server-side auth checks MEDIUM

Recommendations:

  1. Enforce full server-side validation and authorization for every input. Do not rely on client-side zod checks: re-validate emails, roles, file presence/type/size and CSV contents on the server before taking action.
  2. Never log or display raw PII. Remove console.log/console.error that include emails or formState errors in production. Redact or hash emails in logs and use structured logging with different levels (debug/info/warn/error) gated by environment.
  3. Do not expose raw backend error messages to end users. Map internal errors to safe, user-friendly messages. Log full errors in server-side telemetry only with access controls in place.
  4. Do robust server-side file validation: verify MIME type, extension, and inspect file contents. Enforce server-side size limits and reject files > 5MB (or configured limit).
  5. Parse CSVs with a well-tested parser (e.g., PapaParse, csv-parse) on the server to correctly handle quoted fields, commas in values, and different line endings rather than using naive split(',').
  6. Mitigate CSV injection: neutralize leading characters (=, +, -, @) in fields that might be exported back to spreadsheets (prefix with a single quote or escape when generating any CSV for download). Additionally, sanitize any values you echo back to users or include in generated CSVs.
  7. Validate roles server-side and enforce role assignment policies and authorization (e.g., only organization admins can assign 'admin'). Do not trust the client to limit selectable roles.
  8. Limit rate of invite operations and apply authorization checks to the invite endpoints to prevent abuse and privilege escalation.
  9. Avoid sending or rendering untrusted file contents back to other users. If you must present parsed CSV data in the UI, escape it appropriately as text and apply length limits to avoid excessively large cells.

🟡 apps/app/src/app/(app)/[orgId]/people/dashboard/components/EmployeesOverview.tsx (MEDIUM Risk)

# Issue Risk Level
1 User data overfetch: employees includes full user objects MEDIUM

Recommendations:

  1. Limit selected user fields in DB queries instead of including the entire user object. Example: db.member.findMany({ where: { ... }, include: { user: { select: { id: true, name: true /*, other minimal fields */ } } } }).
  2. Map database results to a safe DTO before passing to UI components (strip emails, phone numbers, addresses, or other PII not required by the chart).
  3. Audit EmployeeCompletionChart to ensure it only consumes and renders non-PII/aggregated data. If the chart runs client-side, ensure only sanitized/aggregated values are serialized to the client.
  4. Enforce server-side authorization: verify the requesting session user has permission to list members for the organizationId (ensure session user actually belongs to that organization and has appropriate role).
  5. Add explicit validation/normalization for session.activeOrganizationId (e.g., check type/format such as UUID or numeric id) before using it in queries. Even though the code checks truthiness, explicit validation improves safety and clarity.
  6. Consider using a schema validator (zod/joi) or typed helpers for session payloads so organizationId has a guaranteed type.
  7. For trainingVideos metadata, ensure only required metadata fields are included when sending to the client and that no unexpected sensitive data is present in trainingVideosData.

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

# Issue Risk Level
1 No validation of session.activeOrganizationId before DB query MEDIUM
2 Session-derived orgId used in DB query without explicit auth check MEDIUM
3 member.role used without null/type checks before split/includes MEDIUM

Recommendations:

  1. Validate and assert orgId type and format before using it in queries (e.g., ensure it's the expected string/UUID and not an unexpected value).
  2. Enforce server-side authorization on DB queries: scope queries by the authenticated user (userId) or membership table to ensure the session truly has access to the organization being queried.
  3. Treat session values as sensitive/trust-but-verify data — don't assume integrity without validation. Consider verifying activeOrganizationId is present in the user's organizations list.
  4. Null-check and validate member.role before calling .includes/.split, e.g. ensure typeof role === 'string' and handle empty/null values explicitly.
  5. Add DB/schema constraints (NOT NULL, enum/type for role) to prevent unexpected null/invalid role values, and add runtime guards to avoid crashes if malformed data exists.
  6. Encode/sanitize orgId when interpolating into URLs if there's any chance of unexpected characters (although in most cases validating format is sufficient).
  7. Log and handle unexpected session values gracefully (fail closed), and add tests for session edge-cases to avoid runtime errors.

🟡 apps/portal/src/app/lib/auth.ts (MEDIUM Risk)

# Issue Risk Level
1 Wildcard trustedOrigin 'https://*.trycomp.ai' allows any subdomain MEDIUM
2 Trusted origin includes insecure http://localhost:3000 MEDIUM
3 OTP is 6 digits with 10min expiry; susceptible to brute-force MEDIUM
4 Console.log of NEXT_PUBLIC_BETTER_AUTH_URL may leak env to logs MEDIUM

Recommendations:

  1. Replace wildcard trusted origin with an explicit allowlist of known hosts/subdomains. If wildcard is necessary, ensure strict subdomain ownership and monitoring and consider additional checks (e.g., CSP, cookie host restrictions, and strict SameSite).
  2. Limit 'http://localhost:3000' to development only (guard via NODE_ENV) and ensure no http origins are present in production configuration. Consider building trustedOrigins dynamically from env per-deployment.
  3. Harden OTP usage: shorten expiry (e.g., 2–5 minutes), increase entropy (longer OTP or alphanumeric token), and enforce rate-limiting and intent-based throttling (per-IP and per-account attempt limits, exponential backoff, temporary lockouts after N failures). Consider using single-use signed tokens delivered via email instead of short numeric OTPs.
  4. Remove/avoid logging environment variables or sensitive URLs. Replace console.log with structured, environment-aware logging that redacts sensitive values; ensure logs are not shipped from production with secret values.
  5. (Optional) Although invite IDs appear to be server-generated, consider using unguessable, cryptographically random tokens or signed invitations (HMAC) for links to further reduce risk of enumeration or replay.

🔴 packages/db/prisma/schema/auth.prisma (HIGH Risk)

# Issue Risk Level
1 Private JWKS privateKey stored plaintext in DB HIGH
2 Passwords stored as plain string in Account model HIGH
3 OAuth tokens/idTokens stored plaintext in Account and Session HIGH
4 Roles stored as free-form strings (no DB-level enforcement) HIGH

Recommendations:

  1. Move private keys out of the primary application DB into a KMS/HSM (AWS KMS, GCP KMS, HashiCorp Vault) and do not persist raw privateKey material in the DB. If DB storage is required, use envelope encryption with KMS and restrict access tightly.
  2. Ensure any password value persisted is a secure password hash (bcrypt/argon2/argon2id) with an appropriate cost factor and unique salts; never store user passwords or admin passwords as plaintext.
  3. Do not store raw OAuth access/refresh/id tokens or session tokens in plaintext. Instead: store only token references/hashes (e.g., H(token) using a strong hash like SHA-256 with a secret salt/pepper), or encrypt tokens at rest using field-level encryption/KMS. Limit token lifetime, scope, and retention, and rotate tokens/keys regularly.
  4. Enforce role constraints at the database level where feasible (DB enums or CHECK constraints) in addition to application checks. If a DB-level enum can't be used due to third‑party auth limitations, add runtime validation and strong audit logging for role changes and consider a constrained lookup table for roles with FK references.
  5. Apply principle of least privilege for DB credentials, enable auditing/logging for sensitive tables (jwks, accounts, sessions), use row/column level encryption where supported, and implement regular key/token rotation and access reviews.

💡 Recommendations

View 3 recommendation(s)
  1. Upgrade the ai dependency to >=5.0.52 to address GHSA-rwvc-j5jr-mgvh.
  2. Remove plaintext secret fields from the schema/code: do not persist JWKS privateKey or Account.password as plain strings. Store only public JWKS material or an encrypted/opaque blob and ensure passwords are hashed before insert (e.g., argon2/bcrypt) in application code.
  3. Mitigate injection risks: sanitize or HTML-escape comments before persisting (or ensure all render paths escape), and neutralize CSV formula injection by stripping or prefixing leading characters (=,+,-,@) during server-side CSV parsing/validation.

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

@vercel
Copy link

vercel bot commented Nov 11, 2025

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

Project Deployment Preview Comments Updated (UTC)
app (staging) Ready Ready Preview Comment Nov 11, 2025 11:37pm
portal (staging) Ready Ready Preview Comment Nov 11, 2025 11:37pm

@Marfuen Marfuen merged commit c1a3049 into release Nov 11, 2025
11 checks passed
@claudfuen
Copy link
Contributor

🎉 This PR is included in version 1.57.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants