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.

@comp-ai-code-review
Copy link

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

🔒 Comp AI - Security Review

🔴 Risk Level: HIGH

OSV/npm: xlsx@0.18.5 has two HIGH advisories (Prototype Pollution, ReDoS). ai@5.0.0 has a LOW advisory (filetype whitelist bypass, fixed in 5.0.52). Code shows unvalidated orgId/policyId used directly in DB queries (IDOR/injection).


📦 Dependency Vulnerabilities

🟠 NPM Packages (HIGH)

Risk Score: 8/10 | Summary: 2 high, 1 low CVEs found

Package Version CVE Severity CVSS Summary Fixed In
xlsx 0.18.5 GHSA-4r6h-8v6p-xvw6 HIGH N/A Prototype Pollution in sheetJS No fix yet
xlsx 0.18.5 GHSA-5pgg-2g8v-p4x9 HIGH N/A SheetJS Regular Expression Denial of Service (ReDoS) No fix yet
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 28 file(s) with issues

🟡 apps/api/src/auth/hybrid-auth.guard.ts (MEDIUM Risk)

# Issue Risk Level
1 No validation/sanitization of X-Organization-Id header MEDIUM
2 Detailed error objects logged may leak sensitive data MEDIUM
3 Remote JWKS fetch allows SSRF if BETTER_AUTH_URL is attacker-controlled MEDIUM
4 Very short JWKS cache may enable DoS via frequent key fetching MEDIUM

Recommendations:

  1. Validate and canonicalize X-Organization-Id (e.g., UUID format) before use. Return a 4xx if the header is malformed.
  2. Prefer deriving organization context from the JWT (or require the organization in the JWT) and/or ensure the X-Organization-Id header matches a claim in the token when present.
  3. Avoid logging raw error objects. Log minimal, redacted messages and send structured error IDs to correlate logs without exposing stack traces or sensitive token/JWKS details.
  4. Restrict BETTER_AUTH_URL to a fixed hostname/IP allowlist (validate on startup) and disallow arbitrary URLs to prevent SSRF. Validate the URL scheme and host and reject private/internal IP addresses.
  5. Increase JWKS cache time and implement exponential backoff / rate limiting for JWKS fetches. On retries, avoid forcing zero-cache fetches without rate limiting.
  6. Set strict network timeouts for JWKS fetches and run the JWKS fetcher under limited network privileges (eg. through an outbound proxy that enforces allowlist and rate limits).
  7. Consider including organization id in the JWT (or an organization claim) so the server can trust token-bound org context rather than relying solely on a header.

🟡 apps/api/src/comments/comments.service.ts (MEDIUM Risk)

# Issue Risk Level
1 Stored XSS: comment content not validated/sanitized MEDIUM
2 Unvalidated file uploads: attachments DTO not validated MEDIUM
3 Attachment uploads may occur outside DB transaction causing orphaned files MEDIUM
4 Missing admin privilege enforcement despite comment note MEDIUM
5 Update lacks membership/active-user check before editing MEDIUM
6 Console error logs may expose sensitive info MEDIUM

Recommendations:

  1. Validate and sanitize comment content to prevent XSS
  2. Validate attachments (type, size, virus scan) before accepting
  3. Perform uploads transactionally or cleanup on transaction rollback
  4. Implement and enforce admin/privilege checks as intended
  5. Remove/limit sensitive error logging in production

🟡 apps/api/src/comments/dto/comment-responses.dto.ts (MEDIUM Risk)

# Issue Risk Level
1 Presigned downloadUrl is exposed in API responses MEDIUM
2 User email returned (PII exposure) MEDIUM

Recommendations:

  1. Do not include presigned/signed download URLs directly in response DTOs. Instead, expose only attachment metadata in responses and provide a separate authenticated endpoint to generate a short-lived presigned URL on demand.
  2. If you must return download URLs, ensure they are extremely short-lived, require authentication/authorization to obtain them, and log/monitor access. Prefer server-side generation per-request with authorization checks.
  3. Treat email as PII: omit it from public responses unless the caller is authorized, or return a hashed/obfuscated email or a flag indicating contactability instead of the full email.
  4. Ensure authorization checks are applied wherever these DTOs are returned (e.g., only return Author.email when the requesting user has permission).
  5. Align ApiProperty nullable flags with TypeScript types (e.g., 'deactivated' is marked nullable: true but is typed as boolean — either make its type 'boolean | null' or set nullable: false).
  6. If these DTOs are ever reused for input validation, add class-validator decorators and enable ValidationPipe for incoming request DTOs. (Note: in this file they are response DTOs; validation decorators are not required for outputs.)

🟡 apps/api/src/devices/devices.service.ts (MEDIUM Risk)

# Issue Risk Level
1 No validation for organizationId and memberId inputs MEDIUM
2 Error messages expose internal error.message to callers MEDIUM
3 Logs include organizationId/memberId, risk of sensitive data exposure MEDIUM
4 Unvalidated hostIds from fleetService used directly in getMultipleHosts MEDIUM
5 No pagination or rate limiting when retrieving devices (DoS risk) MEDIUM

Recommendations:

  1. Validate/sanitize organizationId and memberId at the service boundary (or preferably at controller/request DTO level) — e.g., require UUID format, max length, and reject invalid values. Use class-validator or explicit checks before passing to DB calls.
  2. Avoid returning raw internal error messages to clients. Log the full error server-side (this.logger.error(...)) and throw a generic, non-sensitive error to callers (e.g., throw new Error('Failed to retrieve devices')).
  3. Redact or mask sensitive identifiers in logs. Instead of logging full organizationId/memberId, log truncated or hashed values (e.g., first/last N chars or a hash) and include contextual but non-sensitive info.
  4. Validate and constrain hostIds returned from fleetService before passing to getMultipleHosts: ensure each id is numeric and within expected ranges, remove duplicates, and enforce a maximum number of IDs per request. Apply input validation/whitelisting on external service data.
  5. Implement pagination or chunked processing when fetching hosts and host details. Limit the maximum number of hostIds processed per call and use batched calls to getMultipleHosts with timeouts. Add rate limiting on these endpoints (per-organization and per-user) to reduce DoS risk.

🟡 apps/api/src/people/utils/member-queries.ts (MEDIUM Risk)

# Issue Risk Level
1 No input validation on IDs and DTOs before DB use MEDIUM
2 Mass-assignment in updateMember via spread of updateData MEDIUM
3 deleteMember lacks organization check or authorization MEDIUM
4 No authorization checks on create/update/bulk operations MEDIUM
5 bulkCreateMembers inserts without validating user existence MEDIUM
6 Functions return user emails/PII without access controls MEDIUM

Recommendations:

  1. Add server-side validation for all function inputs (organizationId, memberId, DTOs) using a schema validator (e.g., zod, Joi, class-validator). Reject malformed or unexpected values before calling the DB.
  2. Avoid mass-assignment: build update payloads by whitelisting allowed fields rather than spreading updateData. Explicitly map allowed fields (role, department, isActive, fleetDmLabelId) and ignore/throw on extra fields.
  3. Require organization context for destructive operations: change deleteMember to accept organizationId and perform a where { id: memberId, organizationId } check (and/or soft-delete) to prevent cross-org deletion.
  4. Enforce authorization at the boundary (service/controller) and consider adding defensive checks in these DB helpers: verify the caller has permission to create/update/delete members for the given organization.
  5. Validate referenced entities prior to bulk inserts: confirm each userId exists (or let the DB FK enforce it and handle errors). Use DB transactions around createMany + subsequent reads to ensure consistency.
  6. Limit returned PII or ensure callers are authorized to receive it: either restrict user email/name fields returned here or require an explicit flag/permission check before exposing PII.
  7. Add logging/monitoring for bulk operations and destructive actions and consider rate-limiting or additional approval workflows for mass membership changes.
  8. If untrusted input flows into these helpers from HTTP handlers, ensure those handlers validate and sanitize inputs. Prisma parameterizes queries (mitigating SQL injection), but validation is still required for business rules and to prevent abuse.

🟡 apps/api/src/people/utils/member-validator.ts (MEDIUM Risk)

# Issue Risk Level
1 Detailed errors disclose IDs and emails MEDIUM
2 User enumeration via different NotFound/BadRequest responses MEDIUM
3 No input validation for organizationId/userId/memberId MEDIUM
4 No authorization checks in validation methods MEDIUM
5 TOCTOU/race condition risk allowing duplicate members MEDIUM

Recommendations:

  1. Return generic error messages to callers (e.g., "Resource not found" or "Invalid request") and log detailed context (IDs/emails) server-side for debugging. Do not include raw IDs or emails in responses.
  2. Normalize response types and status codes so presence/absence of resources cannot be probed via differing error types or messages. For example, always return 404 for missing resources and 400 for invalid input, independent of internal existence checks.
  3. Validate incoming IDs before database access (e.g., check UUID format with a well-known validator) and reject obviously malformed inputs early.
  4. Enforce authorization checks at the service/handler layer before calling these validation helpers (or accept the calling principal as an argument and validate permissions inside). Ensure authorization is performed consistently for all code paths.
  5. Prevent TOCTOU races and duplicates by enforcing a DB-level uniqueness constraint for members (e.g., unique composite index on (userId, organizationId) with consideration for deactivated flag semantics), and use transactions/upserts where appropriate. Consider using conditional unique indexes or moving deactivation to a separate state so uniqueness is preserved.

🟡 apps/app/src/actions/add-comment.ts (MEDIUM Risk)

# Issue Risk Level
1 Stored XSS risk: comment content saved without sanitization MEDIUM
2 Untrusted header used in revalidatePath (x-pathname/referer) allows abuse MEDIUM
3 No input size/length limits on content, enabling large payloads/DoS MEDIUM
4 No check that entityId belongs to session organization MEDIUM

Recommendations:

  1. Treat content as untrusted. Prefer escaping on render (recommended) or sanitize/normalize input before storing. If allowing HTML, use a secure HTML sanitizer (e.g., DOMPurify server-side or a vetted library) and explicit whitelist.
  2. Do not trust client-controlled headers to determine what to revalidate. Derive the path server-side (for example, from entityType/entityId or a canonical URL stored in DB) or validate/whitelist the header value before passing it to revalidatePath.
  3. Add explicit input constraints to the Zod schema (e.g., z.string().max(2000) or set a reasonable max length) and enforce server-side limits. Combine this with rate limiting to mitigate abuse and large payload DoS.
  4. Before creating the comment, verify the target entity (entityId/entityType) exists and is scoped to session.activeOrganizationId (e.g., a db lookup ensuring the entity.organizationId === session.activeOrganizationId). Reject or error if not found/authorized.
  5. Add logging and monitoring around comment creation and revalidation calls, and apply request rate limiting / abuse protections for comment endpoints.

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

# Issue Risk Level
1 OrganizationId only validated as string (weak validation) MEDIUM
2 User-supplied orgId used directly in DB queries (injection risk) MEDIUM
3 No explicit ctx.user existence check before DB query MEDIUM
4 Headers forwarded to auth.api.setActiveOrganization may expose cookies MEDIUM
5 console.error may leak sensitive internal error details to logs MEDIUM

Recommendations:

  1. Validate organizationId format (e.g., UUID) and length with zod
  2. Sanitize/validate inputs before DB use; enforce typed/parameterized queries
  3. Check ctx.user presence and active status before DB access
  4. Avoid forwarding full request headers; send only required auth token
  5. Log generic errors; store detailed traces in secure telemetry

🟡 apps/app/src/actions/organization/accept-invitation.ts (MEDIUM Risk)

# Issue Risk Level
1 Direct use of inviteCode in DB query (potential SQL injection) MEDIUM
2 inviteCode schema only z.string() — insufficient validation MEDIUM
3 Non-atomic invite acceptance may allow duplicate memberships (race) MEDIUM
4 Throwing caught error via new Error(error as string) may leak internals MEDIUM

Recommendations:

  1. Validate inviteCode more strictly in the input schema (e.g., z.string().uuid() or regex/length checks) so only expected formats are accepted.
  2. Rely on parameterized ORM queries (if using Prisma/ORM) and avoid building raw SQL. If raw queries are used anywhere with inviteCode, convert them to parameterized queries immediately.
  3. Make the invitation-accept flow atomic: wrap member creation and invitation update in a database transaction (or use a DB-side constraint and an upsert pattern). Check for and handle unique constraints to avoid race-created duplicate memberships.
  4. Before writing, re-check invitation status inside the same transaction to avoid TOCTOU races (e.g., ensure invitation.status is still 'pending').
  5. Replace rethrowing of the caught error with throwing a generic error message for clients (e.g., throw new Error('Unable to accept invitation')) and keep full error details in server logs only. When rethrowing, coerce properly (if error is Error) instead of casting arbitrary value to string.
  6. Add unit/integration tests for concurrent invitation acceptance to validate the atomicity and error-handling paths.
  7. Consider defensive checks on ctx.session/ctx.user and ensure session updates are performed inside the transaction where appropriate.

🟡 apps/app/src/actions/organization/get-organization-users-action.ts (MEDIUM Risk)

# Issue Risk Level
1 Missing org membership check: anyone with activeOrganizationId can fetch users MEDIUM
2 Sensitive user data exposure: returns user IDs, names, and images MEDIUM
3 No validation of ctx.session.activeOrganizationId type/value MEDIUM

Recommendations:

  1. Enforce explicit authorization: verify the requesting user is actually a member (or admin) of ctx.session.activeOrganizationId before returning organization user data. For example, query the members table to confirm membership/role, or have authActionClient assert membership at middleware level.
  2. Tighten returned fields: only return the minimum required attributes. Avoid returning raw internal user IDs if not necessary; consider returning a scoped/public identifier. Remove or limit PII (full names) if not required and avoid returning private image URLs (serve signed URLs or thumbnails instead).
  3. Validate and sanitize session organization id: ensure ctx.session.activeOrganizationId is the expected format (e.g., UUID) and type before using it in DB queries. Use a schema validator (zod/joi) or explicit checks.
  4. Add pagination and rate-limiting: implement limit/offset (or cursor) for findMany and enforce per-user rate limits to reduce enumeration risk.
  5. Implement robust logging and monitoring: log authorization failures and DB errors server-side with correlation IDs. Return generic error messages to clients and avoid exposing internal error details.
  6. Apply least-privilege DB queries: prefer selecting only the fields you need (already done for user fields) and consider adding server-side filters for visibility/role-based access if some users should be hidden.

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

# Issue Risk Level
1 ApproverId is client-controlled; no server-side check it matches authenticated user HIGH
2 Stored comment saved without sanitization (stored XSS risk) HIGH
3 No DB transaction — update, email enqueue, and comment can be partial HIGH
4 Assumes member.role is non-null; may throw runtime errors HIGH
5 Potential info leak in logs on catch (exposes internal error details) HIGH

Recommendations:

  1. Enforce approver identity server-side: remove approverId from client-controlled input or validate that approverId === ctx.user.id before performing the publish. Prefer deriving approver from the authenticated session (user.id) so a client cannot impersonate the approver.
  2. Sanitize and validate comment input before storing. At minimum: enforce length limits, strip dangerous HTML tags, or escape content on output. Consider storing raw user input only if you can guarantee proper escaping on every render; otherwise sanitize on write. Use a well-tested sanitizer library (e.g., DOMPurify server-side or an equivalent for your stack) or store and render as plain text with escaping.
  3. Use a DB transaction/outbox pattern: wrap related DB updates (policy update and comment create) in a single transaction so they succeed or fail together. For background jobs (email enqueues), adopt an outbox table or enqueue the job only after the DB transaction commits successfully to avoid partial state (update without comment or emails).
  4. Defensively handle member.role: don't assume it's non-null. Use safe checks (e.g., const roles = (member.role || '').includes(',') ? member.role.split(',') : [member.role || ''];) or validate/normalize role at the DB/schema level so code can rely on a consistent type. Add runtime guards before calling .includes or .split.
  5. Avoid logging raw error objects to console in production. Log minimal context (an error id, high-level message) and capture full error details in a secure error-tracking system (Sentry/Datadog) with access controls. Example: console.error('Error submitting policy for approval: operation failed, errorId=xxxx'); capture error via monitoring rather than printing stack traces to public logs.

🟡 apps/app/src/actions/policies/create-new-policy.ts (MEDIUM Risk)

# Issue Risk Level
1 No role/permission check; any org member can create policies MEDIUM
2 controlIds not validated; may link controls from other orgs MEDIUM
3 Title/description stored without sanitization; risk of stored XSS MEDIUM
4 Relies on external input schema; server-side validation not enforced here MEDIUM

Recommendations:

  1. Enforce RBAC/permission checks before creating policies: verify the member has an explicit role or capability (e.g., policy:create or admin/owner) in the activeOrganizationId context, not just membership.
  2. Validate controlIds belong to the same organization before connecting: query db.control for provided IDs with organizationId = activeOrganizationId and only connect those that match. Reject or error if any requested controlId is not owned by the organization.
  3. Ensure title/description are constrained and escaped: validate length and content in the server-side schema (or here), and sanitize or escape on render. Prefer output escaping in the UI; if HTML is allowed, use a safe HTML sanitizer library (DOMPurify or server-side equivalent) and store a sanitized version or flag for trusted content.
  4. Confirm and harden server-side validation: ensure createPolicySchema (the zod schema) enforces required types, max lengths, and shapes (e.g., controlIds is array of strings). If inputSchema only runs client-side or can be bypassed, add explicit server-side validation here as well.
  5. When connecting records across tables, enforce DB constraints where possible: consider adding organizationId foreign-key constraints or scoped relations to prevent cross-organization linking even if application code is faulty.
  6. Avoid logging sensitive details in error paths; keep returned errors generic and log details to secure logs only.

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

# Issue Risk Level
1 Auth bypass: trusts client-supplied approverId to authorize denial HIGH
2 Stored XSS: comment content saved without sanitization or escaping HIGH
3 Missing state check: no verification of policy status before update HIGH

Recommendations:

  1. Do not trust client-supplied approverId for authorization. Remove approverId from the client input and use ctx.user.id on the server, or explicitly verify ctx.user.id === policy.approverId after loading the DB record before making changes.
  2. Enforce permission/role checks server-side (e.g., verify the member for ctx.user.id in the organization has the approver role and is active) before allowing denial.
  3. Validate policy state before updating (e.g., only allow transition to draft when policy.status === PolicyStatus.changesRequested or whatever the expected 'awaiting approver' state is). Reject the action for unexpected statuses.
  4. Prevent stored XSS: sanitize or escape comment content before saving, or reliably encode it on output. Use a well-maintained sanitizer (or encode on render) and enforce an allow-list of tags if HTML is needed. Also enforce a max length and character constraints for comments.
  5. If entityId in the input schema is unused, remove it from the schema or use it intentionally and validate it. Keep server-side validation strict and only accept fields the server actually uses.
  6. Avoid verbose error logs that may leak sensitive info; return minimal errors to clients and log details in a controlled manner.

🔴 apps/app/src/actions/policies/publish-all.ts (HIGH Risk)

# Issue Risk Level
1 Role check uses substring includes('owner') — auth bypass possible HIGH
2 No DB transaction/rollback — partial policy publishes possible HIGH
3 Detailed logs include stack traces and IDs — sensitive data leak/log injection HIGH
4 organizationId only string-validated; not validated against session.activeOrganizationId HIGH
5 No rate limiting or bulk-send limits — mass-email or DoS risk HIGH

Recommendations:

  1. Replace substring role checks with strict comparisons against a canonical role representation. Example: store roles as enums or arrays and check membership explicitly (e.g., role === Role.owner or roles.includes(Role.owner)) rather than using string.includes().
  2. Wrap the multiple policy updates in a database transaction so updates are all-or-nothing (use your ORM's transaction API). If a single update fails, roll back the transaction to avoid partial publishes.
  3. Reduce sensitive logging: do not log full stack traces or sensitive identifiers (userId, memberId, organizationId) in production logs. If you must log, redact or hash identifiers and avoid including raw error stacks or user-controlled input.
  4. Enforce that parsedInput.organizationId matches session.activeOrganizationId (and validate format). Even though you verify membership in the organization, explicitly checking the activeOrganizationId prevents accidental cross-organization actions and improves intent checks.
  5. Apply rate limits and batch caps to bulk email triggering. Validate and canonicalize recipient emails, cap maximum batch size per request, and consider async queuing with backpressure, retry limits, and monitoring to prevent DoS or abuse.

🟡 apps/app/src/actions/safe-action.ts (MEDIUM Risk)

# Issue Risk Level
1 Rate-limit key uses client-controlled x-forwarded-for header (spoofable) MEDIUM
2 Untrusted headers (referer/x-pathname) used in revalidatePath (path tampering) MEDIUM
3 Raw clientInput and result.data logged to application logs (PII leakage) MEDIUM
4 Server errors are rethrown after logging, risking detailed error exposure to clients MEDIUM
5 organizationId from clientInput used in DB query without validation MEDIUM
6 Audit log persists raw input, IP and userAgent (may include PII) MEDIUM
7 No rate-limit fallback when UPSTASH env absent (rate limiting disabled) MEDIUM

Recommendations:

  1. Do not use client-controllable headers directly as rate-limit keys. Use a server-validated source (e.g., connection IP resolved server-side, or authenticated user ID) and fall back to stable identifiers. If x-forwarded-for must be used, canonicalize and validate it, and combine it with another server-side identifier.
  2. Validate and canonicalize header-derived paths before calling revalidatePath. Maintain a whitelist of allowed path patterns or map referer/x-pathname values to known internal routes. Reject or normalize unexpected values to avoid revalidating arbitrary/attacker-controlled paths.
  3. Redact or omit sensitive fields before logging. Create a sanitization/redaction function that strips PII (emails, tokens, file contents) and apply it to clientInput and result objects before logging. Consider structured logging with explicit allow-lists of safe fields.
  4. Do not rethrow raw Error objects to clients. Log full details server-side but return a generic error message (and safe error codes) to the client. Use an error wrapper that preserves an internal stack for monitoring but sends only non-sensitive messages to callers.
  5. Validate organizationId from clientInput (type, format, length, allowed characters) before using it in DB queries. Prefer deriving organizationId from session/context where possible rather than trusting client input. If client-provided org IDs are required, enforce strict validation and authorization checks.
  6. Audit logs should avoid persisting raw sensitive input. Either store a redacted/hashed representation of inputs or encrypt audit log fields containing PII. Limit stored IP/userAgent detail to what is necessary and consider retention policies and access controls.
  7. Provide a server-side fallback rate-limiter when UPSTASH env vars are absent (e.g., in-memory or another shared store) or fail-open/closed policy. At minimum, explicitly document the behavior when env vars are not set and prefer a safe default (throttle rather than unlimited).

🔴 apps/app/src/app/(app)/[orgId]/frameworks/page.tsx (HIGH Risk)

# Issue Risk Level
1 Unsanitized orgId used directly in DB queries (findUnique/findFirst/findMany) HIGH
2 Cached getControlTasks() is global; may leak tasks across sessions/orgs HIGH
3 Authorization check (member) happens after org onboarding redirect; may leak org info HIGH

Recommendations:

  1. Validate and normalize orgId before using in DB queries (e.g., enforce UUID format, length checks). Treat route params as untrusted input.
  2. Do not rely on a zero-argument cache for per-request/per-organization data. Change getControlTasks to accept organizationId (or session id) and use that as a cache key, or use a per-request cache (pass headers/session or use request-scoped caching).
  3. Perform authorization checks (confirm the current user is a member of the organization and not deactivated) before exposing organization state (onboardingCompleted) or performing other org-scoped reads. Failing closed is safer (return 403 or generic not-found instead of redirecting with org-specific behavior).
  4. Limit the information returned from organization lookups for unauthorised users (e.g., return a generic not-found/403 instead of returning onboarding status), and avoid side-effects (like redirects) before authorization is confirmed.
  5. Add logging and monitoring for unexpected session/header values and for requests that probe different organization IDs; consider rate-limiting org probing endpoints to reduce enumeration risk.
  6. Continue to rely on your ORM's parameterized queries for SQL injection mitigation, but combine that with input validation and authorization checks to prevent logic/authorization flaws.

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

# Issue Risk Level
1 Unsanitized route param used directly in DB queries (orgId) MEDIUM
2 Unvalidated cookie value used as auth token (publicAccessToken) MEDIUM

Recommendations:

  1. Validate and canonicalize params/orgId before DB use. Enforce a strict format (e.g., UUID v4 regex) and/or convert to expected types before passing to db queries.
  2. If possible, rely on ORM parameterization and avoid string interpolation in raw SQL. Add input validation middleware or utility (schema validation with zod/joi) for route params.
  3. Do not trust cookie values as-is for authentication/authorization. Server-validate publicAccessToken (e.g., verify signature, check against DB or token service) before passing to providers or using it to grant access.
  4. Store authentication tokens in secure cookie flags (HttpOnly, Secure, SameSite) and consider signing/encrypting cookie payloads so they cannot be trivially tampered with.
  5. Centralize validation for request inputs (route params, cookies, headers) so rules are consistent across pages/handlers and add unit tests for validation logic.

🟡 apps/app/src/app/(app)/[orgId]/people/all/actions/addEmployeeWithoutInvite.ts (MEDIUM Risk)

# Issue Risk Level
1 Role check uses substring .includes('admin') allowing privilege bypass MEDIUM
2 User inputs (email, organizationId, roles) used directly in DB queries MEDIUM
3 No validation of email format before creating user MEDIUM
4 No whitelist/validation of role values; attacker can request high roles MEDIUM
5 finalUserId may be empty if user creation fails, leading to inconsistent state MEDIUM
6 Creates accounts without email verification, enabling abuse or spam MEDIUM
7 No rate limiting or abuse protection on this endpoint MEDIUM

Recommendations:

  1. Use exact role checks and canonical role representations (prefer enum/array columns). Do NOT rely on substring matching. Example: store roles as an array or enum and check membership exactly (e.g., currentUserMember.roles.includes('admin') if roles is an array, or compare equals on a single role field).
  2. Validate and sanitize inputs: enforce a strict email format (e.g., RFC-like regex or library), validate organizationId format (UUID or DB id), and validate 'roles' against a server-side whitelist of allowed role values before using them.
  3. Whitelist/normalize role values server-side. Never accept arbitrary role strings from the client. Map client-supplied role identifiers to canonical server-side roles and reject unknown values.
  4. Ensure user creation succeeded and finalUserId is non-empty before continuing. Use explicit checks after db.user.create and fail gracefully if creation failed instead of proceeding with an empty id. Consider wrapping user + member creation in a transaction where supported.
  5. Limit privileges for newly created accounts until email is verified (e.g., create user with minimal/no privileges and require verification flow before granting organization roles), or require verification for certain sensitive roles.
  6. Add server-side authorization checks in auth.api.addMember and/or double-check role elevation on the server to avoid relying solely on the caller for role validity.
  7. Add rate limiting, abuse protection and auditing to this API path to mitigate account-creation and mass-invite abuse. Consider CAPTCHA or invitation-only flows for bulk operations.
  8. Log significant events (member reactivation, creation, role changes) with actor and timestamp for auditability.
  9. Sanity-check final data prior to persistence (e.g., ensure role strings are within allowed length/characters) and prefer ORM parameterization (already used) while still validating semantics on the server.

🟡 apps/app/src/app/(app)/[orgId]/people/all/actions/removeMember.ts (MEDIUM Risk)

# Issue Risk Level
1 Unsanitized item/org/user names used in notification emails MEDIUM
2 Member update by id not scoped to organizationId MEDIUM
3 Sessions deleteMany removes all user sessions across orgs MEDIUM
4 Role check uses string.includes; may misclassify roles MEDIUM
5 memberId validated only as string; no UUID/format enforcement MEDIUM

Recommendations:

  1. Sanitize/escape all user-supplied text before including it in emails. For HTML emails, ensure values like organization.name, targetMember.user.name, targetMember.user.email and unassignedItems[].name are escaped or passed through a trusted template engine. Consider an allowlist/encoding step or a library such as DOMPurify (server-side rendering for email) or use safe templating that auto-escapes.
  2. Scope the member update to the organization to avoid accidental cross-org updates. Example: include organizationId in the WHERE clause (or use updateMany with both id and organizationId). Even though you earlier checked the member belongs to the org, adding organizationId in the update is defense-in-depth and prevents race conditions.
  3. Limit session deletions if you only intend to sign the user out of this organization. If sessions are global and you intentionally want to log the user out everywhere, document that behavior. Otherwise, if session rows can be scoped to an organization, include organizationId in the delete condition or implement org-scoped session invalidation tokens.
  4. Use strict role checks rather than string.includes. Store roles as enums or arrays and check membership explicitly (e.g., role === 'admin' or roles.includes('admin')). Avoid substring matching like includes/contains which can misclassify roles (e.g., 'superadmin' matching 'admin').
  5. Validate memberId format in the input schema (e.g., z.string().uuid() or a regex) to catch invalid input early and reduce chances of unexpected behavior. If memberId is not a UUID, enforce whatever canonical format your application uses.
  6. As general hardening: keep server-side authorization checks (already present) but consider logging audited actions with actor id/org id, and add unit/integration tests around edge cases (role strings, race conditions on member transfer) to reduce regressions.

🟡 apps/app/src/app/(app)/[orgId]/people/all/actions/revokeInvitation.ts (MEDIUM Risk)

# Issue Risk Level
1 invitationId only validated as string, not UUID or format MEDIUM
2 Invitation deletion omits organizationId in where clause MEDIUM
3 role property not validated; using .includes may throw or mis-evaluate MEDIUM
4 revalidatePath/revalidateTag use org/user input directly MEDIUM

Recommendations:

  1. Tighten input validation for invitationId: use a stricter schema (e.g., z.string().uuid() or a regex that matches your ID format). This prevents malformed IDs and reduces the attack surface for injection-like issues or accidental misuse.
  2. Include organizationId in the delete call to ensure the invitation being deleted belongs to the current organization. Example: db.invitation.delete({ where: { id: invitationId, organizationId: ctx.session.activeOrganizationId } }) OR use a compound unique key in the ORM if supported. This prevents deletion of records from other organizations if IDs could be guessed or leaked.
  3. Defensively validate/sanitize currentUserMember.role before calling .includes: ensure role exists and is the expected type/shape (string or array). For example: if role is a string, compare explicitly (role === 'admin' || role === 'owner') or normalize to an array first. Add runtime guards to avoid exceptions and logic errors.
  4. Avoid directly interpolating user/session-derived values into revalidation paths/tags without validation or normalization. Ensure activeOrganizationId and user.id are validated/normalized (e.g., match expected UUID format or map to server-known identifiers). If possible, construct revalidation targets from server-side canonical values rather than raw session strings, or encode/escape values before interpolation.
  5. Return precise authorization errors (e.g., 403/permission denied) and avoid logging sensitive data. When catching exceptions, log non-sensitive context and send a generic error message to the client.
  6. As an extra hardening step, consider using transactional/atomic operations: confirm deletion authority and perform delete in a single verified statement where the ORM supports conditional deletes (e.g., delete where id = X and organizationId = Y) to avoid TOCTOU issues.

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

# Issue Risk Level
1 Authorization relies on client-side canManageMembers flag MEDIUM
2 removeMember/revokeInvitation may lack server-side auth checks MEDIUM
3 Full user objects passed to client, may expose sensitive fields MEDIUM
4 Role parsing from comma string isn't strict; may be spoofed MEDIUM

Recommendations:

  1. Enforce authorization on the server for all sensitive actions (removeMember, revokeInvitation): verify the current session, the acting user's membership and roles, and that the acting user belongs to the same organization as the target.
  2. Do not rely on the canManageMembers prop on the client to protect actions. Always perform the same permission checks inside the server actions that perform deletes/revocations and return appropriate 403 responses if unauthorized.
  3. Limit what user data is returned to the client. Replace include: { user: true } with an explicit select of only required fields (e.g., id, email, displayName). Exclude sensitive attributes like password hashes, tokens, or PII not needed by the UI.
  4. Change role storage and checks to a stricter representation (enum/array relation or normalized roles table) rather than a comma-separated string. If you must parse a string, validate and normalize roles against a server-side whitelist before using them for authorization.
  5. Require organizationId and verify it server-side in all member-management actions. Ensure actions validate that the target member/invitation belongs to that organization before performing mutations.
  6. Add server-side input validation and type checks for all inputs to member-management actions (IDs, emails, role values).
  7. Add audit logging for member removals and invitation revocations (acting user, target, organization, timestamp) and rate limiting to slow automated abuse attempts.

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

# Issue Risk Level
1 Owner-role protection enforced only on client-side MEDIUM
2 Unvalidated role input sent to authClient.updateMemberRole MEDIUM
3 Raw server error messages shown in toasts MEDIUM
4 Derived displayName from email without validation MEDIUM
5 Server action result handling trusts client-side shapes MEDIUM

Recommendations:

  1. Enforce owner-role immutability and all role authorization checks server-side in the updateMemberRole endpoint/handler. Client-side checks (like preventing removing 'owner') are only UX — enforce on the authoritative server path and return a safe error message if violated.
  2. Validate and canonicalize role inputs on the server before applying them. Treat client-provided role arrays as untrusted: validate roles against an allowlist, normalize formats (string vs array), and reject unexpected values.
  3. Avoid displaying raw server error strings to end users. Log detailed errors server-side but show generic, non-sensitive messages to users (e.g., 'Unable to update roles. Try again.'). If showing server messages is required, sanitize and map them to user-friendly messages.
  4. Sanitize and validate values derived from user-controlled data (email, name) before using them for display. Although React escapes by default, explicitly validate/normalize displayName (limit length, strip control characters) and ensure downstream components don't render it dangerously (e.g., via dangerouslySetInnerHTML).
  5. Harden client-side handling of server action results: defensively check shapes before accessing nested fields (e.g., verify result && typeof result === 'object' && 'data' in result). Ensure server actions perform authorization checks and return consistent, typed responses. Consider using runtime validation (zod/io-ts) for action responses.
  6. Add server-side authorization and validation for removeMember/revokeInvitation actions (ensure the caller has rights to perform action, ensure resources exist, use consistent error codes).
  7. Log errors internally (with sufficient context) but avoid echoing internal error details back to the client in the response.

🟡 apps/app/src/app/(app)/[orgId]/people/devices/data/index.ts (MEDIUM Risk)

# Issue Risk Level
1 Unsanitized DB-derived IDs used in fleet.get() paths (SSRF/request injection) MEDIUM
2 Unbounded parallel fleet.get() calls for many hosts — DoS/resource exhaustion MEDIUM
3 No error handling for failing fleet requests — exceptions propagate MEDIUM
4 No rate limiting/retries for external API calls MEDIUM

Recommendations:

  1. Validate and sanitize all IDs before interpolating into fleet paths: coerce to Number, validate Number.isInteger(id) && id > 0, and reject/skip anything that doesn't match. Prefer using a whitelist of expected IDs or an allowlist check if possible.
  2. Avoid string-injection into URL paths: ensure path segments are well-formed (for numeric IDs this means strict numeric checks). If you must include non-numeric parts, encode with encodeURIComponent and avoid concatenating raw user/DB strings into URL templates.
  3. Confirm ownership/authorization: verify that each fleet labelId/hostId actually belongs to the organization/session (e.g., a verified mapping in DB, or an explicit fleet API call that verifies the resource belongs to the org) before requesting the fleet endpoints.
  4. Limit concurrency for fleet.get() calls: replace Promise.all(allIds.map(...)) with a bounded-concurrency approach (e.g., p-map, p-limit, a worker pool, or batching) to avoid resource exhaustion.
  5. Add per-request error handling: wrap each fleet.get in try/catch so a single failing request doesn't crash the whole operation. Return partial results and surface errors (with logs) for failed requests.
  6. Implement retry/backoff and rate-limiting for external API calls: use exponential backoff with capped retries for transient errors, and an overall rate limiter (leaky-bucket or token-bucket) to protect the fleet API and your service.
  7. Add telemetry and timeouts: set request timeouts for fleet.get, log slow/failing requests, and surface metrics (success/failure, latency) so you can tune concurrency and retry behavior.
  8. Sanitize and limit responses: when flattening/using host IDs from fleet responses, validate those IDs (numeric check) before issuing further requests.
  9. Consider server-side caching of host details or batching host retrieval endpoints (if fleet supports querying multiple hosts in one call) to reduce number of external requests.

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

# Issue Risk Level
1 User-controlled headers passed to auth.getSession MEDIUM
2 orgId from session used directly in DB query without validation MEDIUM
3 No authorization check verifying access to organization members MEDIUM
4 Parsing member.role as string without validation MEDIUM

Recommendations:

  1. Limit and canonicalize headers passed into auth.api.getSession. Prefer passing only the cookie/header(s) necessary for session resolution (e.g., Cookie) and validate them server-side before use.
  2. Validate orgId before using it in DB queries: assert type (string/UUID), length, and existence. Fail fast if orgId is missing or malformed.
  3. Enforce authorization checks that the current session principal is allowed to list members of orgId. Confirm the session user either belongs to the org or has an allowlisted role/permission. Do not rely solely on presence of an activeOrganizationId field.
  4. Harden member.role handling: validate schema at read time (ensure role is string), guard before calling includes/split (e.g., if (!member.role) return false), and consider normalizing roles to an array in the database or ORM layer to avoid fragile parsing.
  5. Although db.member.findMany appears to use an ORM (safer than raw SQL), ensure all queries remain parameterized and avoid string interpolation into raw SQL. Add explicit error handling around DB operations to avoid leaking internal details.

🟡 apps/app/src/app/(app)/[orgId]/policies/[policyId]/components/RecentAuditLogs.tsx (MEDIUM Risk)

# Issue Risk Level
1 Avatar image URL not validated — data: or SVG URLs may allow script or tracking MEDIUM
2 log.userId.substring used without null/length checks — can throw (runtime DoS) MEDIUM
3 parseLogData trusts shape of log.data without strict validation or type checks (and timestamp used in format without ... MEDIUM

Recommendations:

  1. Avatar URL validation: allowlist schemes (https:), disallow javascript: and untrusted data: URIs and inline SVGs. Prefer serving avatars via a proxy that enforces Content-Type=image/*, strips trackers/embedded content, and normalizes URLs. Set/refine CSP img-src and use referrerpolicy or proxying to reduce tracking.
  2. Defensive rendering for identifiers: check log.userId exists and has sufficient length before calling substring (e.g. userId && userId.length >= 6 ? userId.substring(0,6) : '(unknown)'). Similarly validate timestamp is a Date/number before calling format() and fall back to a safe representation if invalid.
  3. Runtime validation of log.data: validate the shape and types (e.g., using a runtime schema validator like zod or yup) before treating fields like action, details, changes as particular types. Whitelist acceptable action values and coerce/normalize changes so code can safely toString() values or render typed diffs.
  4. Server-side normalization/sanitization: ensure audit log payloads are normalized and validated server-side before storing or sending to clients. Reject or canonicalize unexpected types to avoid client runtime errors.
  5. Use defensive UI coding: avoid assumptions about optional relations (user, member, timestamps). Provide safe fallbacks and avoid calling methods on possibly undefined values.
  6. Deploy/strengthen CSP to mitigate image-based or other client-side risks and reduce blast radius of any incorrect content handling.

🔴 apps/app/src/app/(app)/[orgId]/policies/[policyId]/data/index.ts (HIGH Risk)

# Issue Risk Level
1 Missing authorization checks before returning policy-related data HIGH
2 IDOR: direct use of policyId to fetch resources within org HIGH
3 No validation or sanitization of policyId before DB queries HIGH
4 Exposure of PII: returns user email/name/image without access control HIGH
5 Attachment URLs returned directly without access checks or signed URLs HIGH

Recommendations:

  1. Enforce per-user authorization checks beyond organization scoping: verify the caller is a member of the organization and has the appropriate role/permission for the requested operation (read/approve/assign). E.g., check session user id against membership and role before returning policy, comments, or attachments.
  2. Add object-level permission checks to prevent IDOR: do not rely solely on presence of organizationId. Ensure the session user is authorized to access the specific policy (or control list) in question.
  3. Validate and canonicalize policyId (and any other incoming IDs). For example, reject inputs that are not valid UUIDs (if IDs are UUIDs), enforce length/charset limits, and return 4xx for invalid IDs. This reduces risk and improves error handling.
  4. Minimize returned PII. Only return user fields the caller is authorized to see. Consider returning less identifying info (e.g., name only or anonymized info) or include the user's permission to view PII in the response contract.
  5. Serve attachments via an authenticated endpoint or produce signed, time-limited URLs (e.g., presigned S3 URLs) instead of returning raw stored URLs. Validate that the requesting user has access to the attachment before generating/returning a URL.
  6. Add rate limiting and pagination/limits for list endpoints. The logs function already uses take: 3 (good) — apply similar limits to comments/controls if large sets are possible.
  7. Log and monitor authorization failures and unexpected access patterns. Return generic errors for unauthorized access to avoid leaking existence of objects.
  8. Although ORM parameterized queries mitigate SQL injection risk, keep input validation and avoid concatenating user inputs into raw queries. If any raw queries are used elsewhere, ensure parameterization.

🔴 apps/app/src/app/(app)/[orgId]/risk/[riskId]/page.tsx (HIGH Risk)

# Issue Risk Level
1 React cache used with headers/session leads to cross-request data leakage and auth bypass HIGH
2 getAssignees cached with no args can expose member lists to other sessions HIGH

Recommendations:

  1. Do not call react's cache() on functions that read per-request headers() or session data unless the cache key incorporates request-specific information. Either remove cache() from getAssignees and from getRisk (or at least the parts that depend on headers/session), or refactor to pass the session/org id explicitly as a parameter so it becomes part of the cache key.
  2. Change getAssignees to accept an explicit organization/session id argument (e.g., getAssignees(orgId)) and only cache results per-org or per-authorized-scope. Example: cache(async (orgId) => { ... }) and call getAssignees(session.session.activeOrganizationId).
  3. For getRisk, ensure the cache key includes riskId and the session organization id (e.g., cache by (riskId, orgId)) so cached results are not reused across different sessions/orgs.
  4. If caching user-sensitive data is required, implement per-session or per-organization scoped caches (explicit keys) and enforce short TTLs or manual invalidation on permission changes.
  5. Add defense-in-depth: validate the format of riskId (e.g., UUID check) before use to catch malformed values early (this is not a SQL-injection fix but improves robustness).
  6. Audit other server-side cached helpers and any components (Comments, RiskActions, etc.) to ensure they do not read headers/session inside cache() without keys, and verify all data returned is properly authorized per-session.
  7. Log and monitor unexpected cache hits that return organization-scoped data to detect cross-tenant leaks.

🟢 apps/app/src/app/(app)/[orgId]/tasks/components/TaskList.tsx (LOW Risk)

# Issue Risk Level
1 Unsanitized orgId used as localStorage key LOW
2 URL query-state (status/assignee) used without validation LOW
3 Props (initialTasks/members) accepted without schema validation LOW
4 Parsing run.createdAt directly may accept invalid dates LOW
5 LocalStorage could be manipulated leading to UI inconsistencies LOW

Recommendations:

  1. Normalize and validate orgId before using it as a localStorage key. Prefer a strict whitelist or pattern (e.g., /^[a-z0-9_-]{1,64}$/i) or apply encodeURIComponent to avoid control characters. Treat localStorage keys as untrusted input when used to change app behavior.
  2. Validate query-state values (status and assignee) against expected sets before using them. For example, coerce / check status against the known statuses array and ensure assignee matches an eligibleAssignees id. Reject or fall back to a safe default for unknown values.
  3. Add runtime schema validation for props coming from external sources (server/parent) using a library like zod or io-ts. Validate initialTasks, members, and evidenceAutomations shapes (including nested runs) and reject/normalize unexpected shapes before using them in UI logic.
  4. Harden date handling: validate run.createdAt before using new Date(...). Use Date.parse or a date library (dayjs, luxon) and check isValid() or isNaN(getTime()) and handle invalid dates gracefully (skip or default).
  5. Do not rely on client-side localStorage for any security-critical state. Sanitize and validate read values and guard against tampered values (e.g., track lastLoadedOrgId separately and defensively handle malformed values).
  6. Add defensive checks where values are used (e.g., guard against null/undefined/NaN when computing stats and ensure numeric counters are integers). Log or surface errors for invalid data shapes to avoid silent misbehavior.

💡 Recommendations

View 3 recommendation(s)
  1. Upgrade vulnerable deps: update xlsx (0.18.5) to a patched release referenced by the GHSA advisories and update ai to >=5.0.52 (per the advisory). Rebuild lockfile and pin versions to avoid reintroducing the vulnerable ranges.
  2. Add strict input validation for all external IDs before any DB or URL usage: validate route params/session values (orgId, policyId, memberId, invitationId) with a schema validator (e.g., zod/class-validator) enforcing UUID or numeric formats, length limits, and reject malformed values at the controller boundary.
  3. Enforce object-level authorization and scoped DB operations: include organizationId in WHERE clauses for updates/deletes/reads (e.g., where: { id: X, organizationId: session.activeOrganizationId }), and verify resource.organizationId matches the session before returning attachments/PII to prevent IDOR/injection misuse.

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

@CLAassistant
Copy link

CLAassistant commented Nov 21, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ Marfuen
❌ github-actions[bot]
You have signed the CLA already but the status is still pending? Let us recheck it.

@vercel
Copy link

vercel bot commented Nov 21, 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 24, 2025 5:51pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
portal (staging) Skipped Skipped Nov 24, 2025 5:51pm

* feat(db): add deactivated column to member table

* fix(db): publish new db version: 1.3.16

* fix(api): remove access from deactivated members

* fix(app): remove access from deactivated members

* fix(portal): remove access from deactivated members

* fix(app): make member deactivated when removing

* fix(api): include deactivated value to comments API response

* fix(app): show alert icon for deactivated users on RecentLogs and comments

* fix(db): remove duplicated migration script for user deactivation

* fix(app): reinvite the deactivate employee

* feat(app): send an email to owner when the user is an assignee when removing a member

---------

Co-authored-by: chasprowebdev <chasgarciaprowebdev@gmail.com>
Co-authored-by: Mariano Fuentes <marfuen98@gmail.com>
@vercel vercel bot temporarily deployed to staging – portal November 24, 2025 17:48 Inactive
@Marfuen Marfuen merged commit a0406af into release Nov 24, 2025
11 of 12 checks passed
@claudfuen
Copy link
Contributor

🎉 This PR is included in version 1.63.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.

4 participants