Skip to content

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Nov 4, 2025

This is an automated pull request to merge chas/deactivate-member into dev.
It was created by the [Auto Pull Request] action.

@vercel
Copy link

vercel bot commented Nov 4, 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 21, 2025 9:55pm
portal Ready Ready Preview Comment Nov 21, 2025 9:55pm

@comp-ai-code-review
Copy link

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

🔒 Comp AI - Security Review

🔴 Risk Level: HIGH

OSV finds xlsx@0.18.5 (GHSA-4r6h-8v6p-xvw6, GHSA-5pgg-2g8v-p4x9) and ai@5.0.0 (fix in 5.0.52). Code shows unvalidated IDs and client-trusted approverId/header values leading to injection/IDOR risks.


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

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

# Issue Risk Level
1 Unvalidated BETTER_AUTH_URL allows non-HTTPS JWKS fetches MEDIUM
2 Remote JWKS fetch uses configurable URL (SSRF if config is attacker-controlled) MEDIUM
3 Retry logic forces fresh JWKS fetches (DoS risk against JWKS endpoint) MEDIUM
4 DB query uses userId and organizationId directly; risk if ORM not parameterized MEDIUM
5 Detailed error messages leak internal state and connectivity info MEDIUM
6 Console logs may expose sensitive verification errors in logs MEDIUM

Recommendations:

  1. Enforce and validate BETTER_AUTH_URL: require https scheme and restrict to a whitelist of allowed hostnames (or inject full JWKS URL from immutable config). Reject or fail startup when an insecure or unexpected URL is provided.
  2. Harden JWKS usage: validate the resolved JWKS URL hostname against an allowlist; avoid fetching arbitrary URLs derived from attacker-controllable config.
  3. Add backoff/rate-limiting to JWKS fetch/retry logic: limit retries per token verification, add exponential backoff and circuit-breaker behavior and overall rate-limiting on the auth endpoint to mitigate DoS.
  4. Sanitize/validate all headers and JWT claims before use. Although the code verifies membership for X-Organization-Id, ensure X-Organization-Id is validated (format/length) and consider canonicalizing IDs prior to DB queries.
  5. Treat DB access as sensitive: confirm the ORM produces parameterized queries (document or assert the used ORM/DB layer does parameterization) and add input validation on values used in queries.
  6. Return generic error messages to clients. Keep detailed diagnostic logs only in secured server logs and avoid including connectivity internals or sensitive token details in logs returned to users.
  7. Reduce logging verbosity for verification failures and ensure logs containing sensitive information are written to protected storage with access controls and retention policies.

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

# Issue Risk Level
1 No input sanitization for comment content (stored XSS risk) MEDIUM
2 Attachments lack server-side validation (type/size/virus) MEDIUM
3 File uploads performed inside DB transaction can be orphaned on rollback MEDIUM
4 Error logs may expose sensitive/internal data MEDIUM

Recommendations:

  1. Sanitize or HTML-encode comment content before rendering
  2. Validate attachments: whitelist MIME, max size, virus-scan files
  3. Upload files outside DB transaction; implement cleanup on rollback
  4. Redact sensitive data from logs and avoid logging raw errors
  5. Add rate-limiting and monitoring on comment endpoints

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

# Issue Risk Level
1 Signed downloadUrl included in API response (sensitive URL exposure) MEDIUM
2 User email and image returned in responses (PII exposure) MEDIUM

Recommendations:

  1. Do not embed pre-signed download URLs directly in publicly returned DTOs. Instead return attachment metadata and provide a protected server endpoint that generates/returns a pre-signed URL on demand after authorization checks.
  2. If pre-signed URLs must be returned, ensure they are extremely short-lived, scoped to the minimum permissions needed, and logged/audited. Rotate signing credentials and minimize query-string leakage.
  3. Enforce authorization before returning any attachment download URL or PII. Return download URLs only to authenticated/authorized principals and consider additional checks (e.g., resource ownership, role-based access).
  4. Limit exposure of PII: omit email and profile image unless the requester is authorized to view them, or return masked/partial values (e.g., user@••••.com) or an opaque user ID. Provide a separate endpoint that returns full profile data only to authorized callers.
  5. Ensure request-side DTOs (not shown in this file) use class-validator/class-transformer decorators to validate inputs. Although the current file contains response DTOs, you should still validate all incoming request DTOs.

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

# Issue Risk Level
1 Missing validation of organizationId/memberId before DB queries MEDIUM
2 User-provided IDs used directly in DB queries (possible SQL injection) MEDIUM
3 No authorization check to ensure caller can access org or member data MEDIUM
4 Internal error messages logged/returned, may leak sensitive info MEDIUM

Recommendations:

  1. Validate and sanitize organizationId and memberId at the API boundary (e.g., use DTOs and class-validator in NestJS). Enforce accepted formats (UUIDs or numeric IDs) and return 400 for invalid formats.
  2. Ensure DB queries use parameterized ORM methods (Prisma/TypeORM do this by default). Even so, validate inputs and avoid building raw SQL with user input. If raw queries are needed, always use parameter binding.
  3. Add authorization checks (NestJS guards/interceptors) to enforce that the caller is permitted to access the requested organization/member. Implement role-based checks and verify the requesting user’s membership/role in the organization before returning data.
  4. Do not return raw error.message to callers. Throw generic HttpExceptions with safe messages (e.g., 'Failed to retrieve devices') and log detailed errors to secure logs only. Avoid logging entire error objects to public logs; redact sensitive fields.
  5. Limit and validate hostIds before calling fleetService.getMultipleHosts: cap the maximum number of IDs, ensure they are the expected type (numbers), and handle excessively large lists gracefully.
  6. Harden fleetService interactions: wrap external service calls in try/catch, validate and sanitize responses before returning them to clients, and implement timeouts/retries and rate-limiting for external requests.
  7. Audit logging: ensure logs are stored securely, rotate logs, and avoid leaking PII or stack traces to user-facing responses.
  8. Add unit/integration tests to cover malformed IDs, unauthorized access, and error paths to ensure protections remain effective.

🔴 apps/api/src/people/utils/member-queries.ts (HIGH Risk)

# Issue Risk Level
1 deleteMember lacks organization check; can delete members across orgs HIGH
2 updateMember lacks organization check; can update members across orgs HIGH
3 Mass assignment in updateMember: updateData spread permits arbitrary fields HIGH
4 No input validation for IDs (memberId, organizationId, userId) before DB use HIGH

Recommendations:

  1. Require and verify organizationId on update and delete operations. Change db calls to include organizationId in the where clause (e.g., where: { id: memberId, organizationId }) so updates/deletes are scoped to the org.
  2. Enforce authorization checks before performing DB operations: validate the caller's organization membership/role (from request context/session) and ensure they have permission to modify the target member.
  3. Whitelist allowed update fields instead of spreading updateData directly. Build an explicit payload with only permitted properties (e.g., role, department, isActive, fleetDmLabelId) and ignore unexpected fields to prevent mass-assignment.
  4. Validate and sanitize IDs and other inputs before using them in DB queries. Enforce expected formats (e.g., UUID v4) using a library or strict regex and return 4xx on invalid input.
  5. Add audit logging for destructive operations (update/delete/bulk create) so actions can be traced to a principal, and consider wrapping multi-step or bulk operations in a transaction when needed.
  6. Consider adding unit/integration tests that assert org-scoped behavior (e.g., attempts to update/delete a member outside the caller's org should fail).

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

# Issue Risk Level
1 User enumeration via detailed error messages (exposes emails/IDs) MEDIUM
2 Missing input validation on IDs used directly in DB queries MEDIUM
3 Race condition between membership check and creation (TOCTOU) MEDIUM

Recommendations:

  1. Do not include identifying information in error messages returned to callers. Replace messages containing raw IDs or emails with generic messages (e.g., 'Resource not found' or 'Unable to complete request'); log detailed info server-side for diagnostics.
  2. Validate and enforce ID formats (e.g., UUID checks) before issuing DB queries. Use schema/DTO validation (class-validator or similar) or a shared validation util to reject malformed IDs early.
  3. Enforce uniqueness at the database level for membership (e.g., unique constraint on (organizationId, userId)) and perform member creation inside a transaction or use an atomic upsert to avoid TOCTOU race conditions. Additionally, catch and handle unique-constraint errors (ORM-specific, e.g., Prisma P2002) and map them to user-friendly responses.
  4. Add rate limiting and monitoring on endpoints that use these validators to reduce the effectiveness of enumeration attempts; log suspicious activity for investigation.
  5. Sanitize and type-check inputs (length checks, character whitelist) before handing them to the ORM even though the ORM parameterizes queries. Keep server-side validation consistent with DTOs and API schemas.
  6. If returning any user-specific details is necessary for internal APIs, ensure those endpoints are fully authenticated/authorized and not exposed to unauthenticated callers.

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

# Issue Risk Level
1 Persistent XSS via unsanitized comment content MEDIUM
2 Missing validation for content length/format and entityId MEDIUM
3 Unvalidated revalidatePath using client headers (x-pathname/referer) MEDIUM
4 Client-controlled header may trigger arbitrary route revalidation MEDIUM

Recommendations:

  1. Sanitize or HTML-encode comment content before storing OR ensure all rendering paths escape/encode content. Consider using a well-known sanitizer (DOMPurify / sanitize-html) with an allowlist if HTML is allowed, or strip HTML entirely and store plain text.
  2. Tighten the input schema: enforce min/max length for content (e.g., z.string().min(1).max(2000)), and validate entityId format (e.g., UUID via z.string().uuid() or a regex) to prevent overly long inputs and malformed ids.
  3. Do not trust client-controlled headers for determining revalidation paths. Derive the path server-side from known server state (e.g., based on entityId/entityType and canonical route generation) rather than using x-pathname or referer.
  4. If revalidatePath must accept an external input, canonicalize and validate it against an allowlist of permitted routes/patterns (exact matches or strict regexes). Reject or log attempts to revalidate unrecognized paths.
  5. Remove or harden the simple locale strip (path.replace(//[a-z]{2}//, '/')) — it is insufficient as validation. Use a robust parser to normalize paths and then verify against allowed prefixes.
  6. Add authorization checks around revalidation (ensure the requesting user is allowed to trigger revalidation for the target resource) and add rate limiting / throttling to revalidation endpoints to mitigate abuse.
  7. Log suspicious header-derived revalidation attempts and monitor for abnormal patterns (mass revalidations, varied paths) to detect abuse.

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

# Issue Risk Level
1 Insufficient validation: organizationId only validated as z.string MEDIUM
2 No check that ctx.user exists before using user.id MEDIUM
3 Forwards full request headers to auth.api, may leak auth tokens MEDIUM
4 Potential info leak via console.error dumping full error MEDIUM
5 No granular role/permission check; membership alone used MEDIUM

Recommendations:

  1. Tighten input validation for organizationId: require a strict format (e.g., z.string().uuid() or a constrained regex + length checks) so consumers and downstream DB/logic can rely on a known identifier shape.
  2. Defensive authentication check: even if middleware (authActionClient) normally sets ctx.user, add an explicit runtime guard (if (!ctx?.user) return { success: false, error: 'Unauthorized' }) or ensure the action throws/returns 401 early to avoid runtime TypeErrors and accidental unauthenticated access.
  3. Do not forward all incoming headers to auth.api.setActiveOrganization. Instead use a server-to-server credential or a scoped service token, or explicitly forward only the minimal required auth header. Avoid relaying client-sent authorization headers or cookies verbatim.
  4. Replace console.error with structured logging that sanitizes sensitive fields (no full error stacks in logs sent to stdout). Use your centralized logger/telemetry with redaction rules and ensure sensitive tokens/PII are stripped before logging.
  5. Enforce authorization beyond membership: check member.role/permission fields (e.g., isAdmin/isOwner/canSwitchOrg) before allowing organization switching. If the db.member model lacks role metadata, add it and validate here.
  6. Add auditing: record who switched the active organization and when (audit log entry) and ensure revalidatePath usage is safe and rate-limited if necessary.
  7. Consider additional hardening: rate-limit this action (if not already), validate organization existence earlier, and ensure DB calls use ORM parameterization (looks like an ORM is used here) to mitigate injection risks.

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

# Issue Risk Level
1 No validation on inviteCode format or length MEDIUM
2 No brute-force protection for invite-code submissions MEDIUM
3 Race condition creating member (TOCTOU) may allow duplicates or errors MEDIUM
4 Console.error may log sensitive invitation or user data MEDIUM
5 Rethrowing raw error object may disclose internal details to clients MEDIUM
6 Unsanitized organizationId used to build redirect path MEDIUM

Recommendations:

  1. Enforce stricter inviteCode validation in the input schema (use zod pattern for expected format, set a max length).
  2. Add rate-limiting / throttling on the invitation acceptance action (IP- and user-based limits, exponential backoff) or require a short-lived one-time token tied to the session.
  3. Use a database-level uniqueness constraint and perform acceptance inside a transaction or use an atomic upsert to avoid TOCTOU/duplicate member creation. Check for membership existence in the same transaction that creates the member (or handle unique-constraint violations gracefully).
  4. Avoid logging full error objects or sensitive values. Log minimal contextual identifiers and use structured logging with redaction for sensitive fields.
  5. Return sanitized/generic error messages to clients (do not surface raw error strings). Preserve full error details only in server logs that are properly access controlled.
  6. Validate and normalize organizationId before building redirects (ensure it matches an expected pattern/format). Prefer constructing redirects using internal routing helpers rather than concatenating potentially untrusted values into paths.

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

# Issue Risk Level
1 Auth bypass: trusts session.activeOrganizationId without verifying membership MEDIUM
2 Missing validation: activeOrganizationId not validated before DB query MEDIUM
3 Excessive disclosure: returns all org users without pagination or role filtering MEDIUM
4 Missing validation: parsedInput not validated or used MEDIUM

Recommendations:

  1. Verify requester is a member/has role in the organization before querying
  2. Validate activeOrganizationId format (e.g., UUID) server-side
  3. Add pagination/limits to queries to avoid bulk data exposure
  4. Select/mask only required user fields to reduce sensitive data exposure
  5. Add rate limiting and audit logging for org user access

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

# Issue Risk Level
1 Auth bypass: client can accept by supplying approverId HIGH
2 Client-supplied approverId trusted; not verified against authenticated user HIGH
3 Missing sanitization of comment leading to stored XSS HIGH
4 TOCTOU: policy read-then-update without DB transaction HIGH

Recommendations:

  1. Do not trust client-supplied approverId. Derive the approver from the authenticated session (e.g., require approverId === user.id) or server-side lookup of the authorized approver for this action.
  2. Enforce explicit authorization server-side: verify the authenticated user has the approver role/permission for this policy before changing status. Example: check membership/role or permission, or compare policy.approverId against the member id associated with the session user.
  3. Make the publish/accept operation atomic. Either perform an update constrained by approverId in the WHERE clause (e.g., update where id=X AND organizationId=Y AND approverId=Z) or wrap read+update logic in a DB transaction so state cannot change between read and update. Return an error if the conditional update affects 0 rows.
  4. Sanitize or escape comment content before storing or ensure comments are escaped on render. Use a proven sanitizer (e.g., DOMPurify server-side, sanitize-html, or HTML-escape on output) and enforce length/character limits.
  5. Remove reliance on client-sent approverId completely where possible: accept action only if the session user is the approver, or use a secure server-side confirmation workflow (e.g., action triggered by the approver's authenticated session or by an email link with one-time token).
  6. Add logging and monitoring for approve/publish actions so suspicious attempts (different user submitting approverId of another user) are auditable and can be blocked/alerted.

🔴 apps/app/src/actions/policies/create-new-policy.ts (HIGH Risk)

# Issue Risk Level
1 No ownership check for controlIds allows linking controls across organizations HIGH
2 controlIds not fully validated before DB connect (type/format/length) HIGH
3 Console.error logs raw error objects, may leak sensitive data HIGH
4 No server-side limits on title/description may allow large payloads HIGH

Recommendations:

  1. Verify each controlId belongs to activeOrganizationId before connecting
  2. Enforce and validate controlIds schema: types, max length, and dedupe
  3. Enforce title/description length limits and sanitize input
  4. Avoid logging raw error objects in production; use structured logs
  5. Add rate limiting and audit logging for createPolicyAction

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

# Issue Risk Level
1 Auth bypass: client-supplied approverId lets non-approver deny policies HIGH
2 Stored XSS: comment saved without sanitization or encoding HIGH

Recommendations:

  1. Stop trusting approverId from the client. Derive the approver on the server from the authenticated context (e.g., look up the member record for ctx.user.id and use its id) or require that approverId matches the current authenticated member id before allowing the action.
  2. Enforce server-side authorization: verify the current user is actually the assigned approver (or has the required role/permission) for the policy. Example checks: find member by user.id & org id, confirm member.id === policy.approverId or member has approver role.
  3. Perform the policy update and comment creation in a single transaction so updates cannot be partially applied by a malicious request.
  4. Sanitize or canonicalize comment content before storing (server-side) using a robust HTML sanitizer (e.g., sanitize-html, DOMPurify on server) or store raw text but ensure every renderer HTML-escapes it. Prefer escaping on render as a defense-in-depth measure.
  5. If rich text/HTML is required, sanitize input to an allowlist of tags/attributes and remove scripts, event handlers, style attributes, and dangerous URL schemes.
  6. Validate comment length and content (e.g., max length, disallow control characters) before saving.
  7. Treat stored content as untrusted when rendering: ensure templates or UI components escape content by default; avoid dangerouslySetInnerHTML unless content is sanitized and trusted.
  8. Limit logging of sensitive details. Avoid logging full input payloads; log only necessary identifiers and high-level error messages.
  9. Return minimal error information to clients (avoid echoing internal error stacks).

🟡 apps/app/src/actions/policies/publish-all.ts (MEDIUM Risk)

# Issue Risk Level
1 Insufficient validation for organizationId MEDIUM
2 No DB transaction: partial policy publishes on error MEDIUM
3 Sensitive info logged (error stack, userId, memberId, policyName) MEDIUM
4 Role check uses includes(), may allow incorrect matches MEDIUM
5 Email inputs (email/name) not validated before bulk send MEDIUM
6 No rate limiting/abuse protection for bulk publish action MEDIUM

Recommendations:

  1. Tighten input validation: validate organizationId as a UUID (or whatever canonical format you use) in the Zod schema (e.g. z.string().uuid() or a refinement) and validate other inputs early.
  2. Make policy status updates atomic: wrap the updates in a DB transaction (or use updateMany if appropriate) so either all policies are published or none are. If per-row assignee/reviewDate must be set, use a transaction (db.$transaction / prisma transaction).
  3. Avoid logging sensitive data in production: remove or redact stacks, userId, memberId, policyName, and other PII from logs (or only log them at debug level behind a guard). Consider structured logging with safe fields and a separate secure store for full traces.
  4. Use strict role checks: compare role enums/values exactly (e.g. member.role === Role.owner or stored role array includes('owner') with rigorous type checks). Avoid substring matching (string.includes) for role checks.
  5. Validate and sanitize email/name before sending: use a robust email validator and sanitize names (escape HTML/newlines) to prevent header injection or content injection in emails. Ensure sendPublishAllPoliciesEmail enforces/validates inputs server-side.
  6. Add rate limiting / abuse protection: add server-side throttling per-user and per-organization for expensive bulk actions. Add authorization checks and audit logging for bulk operations.
  7. Improve error handling: when a single policy update fails, avoid re-throwing raw errors that surface stacks and sensitive context. Return a clear, non-sensitive error to callers and record detailed info in a secure/controlled audit log when needed.

🔴 apps/app/src/actions/safe-action.ts (HIGH Risk)

# Issue Risk Level
1 Sensitive input/result logged (may include secrets/PII) HIGH
2 Audit log stores raw client input (possible sensitive data) HIGH
3 Rate limit key uses spoofable x-forwarded-for header HIGH
4 Rate limit keyed by client-provided metadata.name (evasion) HIGH
5 RevalidatePath uses unvalidated header path (open revalidation) HIGH
6 Server errors thrown to client may leak internal details HIGH
7 Metadata and headers not validated before use HIGH
8 entityId parsing not validated (incorrect mapping risk) HIGH

Recommendations:

  1. Redact or omit sensitive fields before logging. Use a allowlist of safe fields or a redact function to remove PII/secrets from clientInput and results before logger.info/logger.warn.
  2. Do not store raw client input in audit logs. Sanitize/normalize or store only an allowlisted subset of input fields. Consider hashing or encrypting sensitive fields, or omitting them entirely from audit entries.
  3. Use a proxy-verified IP (e.g., provider supplied header validated by your infrastructure) or server-side connection IP for rate limiting. Do not rely directly on client-controllable X-Forwarded-For without verification.
  4. Do not include client-controlled metadata.name in the rate-limit key. Use a server-assigned action identifier (whitelist) or a stable server-side mapping for rate-limit keys to prevent client evasion.
  5. Validate and whitelist paths before calling revalidatePath. Do not pass raw Referer or custom x-pathname header values directly to revalidatePath. Normalize and ensure the path is within your allowed set.
  6. Return generic error messages to clients. Log full error details server-side. Modify handleServerError to throw a generic error to the client (and keep stack in server logs) instead of re-throwing the original Error object.
  7. Although metadata has a schema, validate all metadata and header-derived values before use. Explicitly validate/normalize headers (ip, user-agent, x-pathname) and enforce allowed formats/lengths.
  8. Validate entityId format before parsing. Enforce a strict regex or schema for entityId, and handle unexpected formats gracefully (do not rely on splitting alone). Consider mapping only known prefixes and log/ignore unknown formats.

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

# Issue Risk Level
1 No authorization check before fetching organization data HIGH
2 getControlTasks() is cached with no args, can leak another user's tasks HIGH
3 organizationId/params used directly in DB queries without validation HIGH
4 Session-derived activeOrganizationId used without verifying membership HIGH

Recommendations:

  1. Perform authorization checks before any organization-scoped DB calls: ensure the requesting user is a member of the org and has the required role/permissions prior to fetching org data (including onboarding status).
  2. Avoid caching per-user or per-tenant data with a global/no-arg cache. Either remove caching for getControlTasks or include a cache key that includes a stable per-request/tenant identifier (e.g., session.user.id or organizationId) and ensure the key cannot be forged.
  3. Validate and sanitize route params (orgId) server-side before using them in queries. Enforce expected formats (e.g., UUID) and fail early with 4xx responses for invalid values.
  4. Do not rely solely on session.session.activeOrganizationId for access decisions. Explicitly verify membership (e.g., db.member.findFirst) and required permissions/roles before returning org-scoped data or allowing operations.
  5. Return appropriate HTTP status (403) or redirect when a user is not authorized to access the organization, instead of continuing to fetch or render org data.
  6. Audit other cached or memoized functions for use of request-specific, user-specific, or tenant-specific data to avoid cross-user/tenant data leakage.

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

# Issue Risk Level
1 Unvalidated orgId route param used directly in DB queries MEDIUM
2 publicAccessToken read from cookie and used without verification MEDIUM
3 Cookies and headers used without explicit validation or sanitization MEDIUM

Recommendations:

  1. Validate and normalize params.orgId before using in DB calls (e.g., enforce UUID format or a strict pattern, reject invalid values). Fail fast and return 404/400 for invalid IDs.
  2. Do not trust tokens from client-writable cookies for authorization decisions without server-side verification. If the cookie contains an access token intended to be trusted, validate it server-side (e.g., call auth.verify or introspect the token) before granting access or using it to initialize privileged SDKs.
  3. Mark access tokens as HttpOnly and Secure where possible and scope them narrowly (short TTL, least-privilege scopes). Consider signing/encrypting any client-settable cookie values so you can detect tampering.
  4. Limit the use of unvalidated cookie values to UI-only behavior (e.g., sidebar collapse state). For any behavioral or auth-sensitive flows, derive state server-side from trusted sources.
  5. When using an ORM (e.g., Prisma), prefer parameterized query APIs (the standard ORM methods are usually safe). Avoid raw SQL string concatenation with user inputs. Use least-privilege DB roles and column-level permissions where applicable.
  6. Avoid logging sensitive auth state (e.g., 'no session') in production, and ensure logs do not contain tokens, PII, or identifiers that could leak sensitive information.
  7. Add explicit validation/typing on headers you forward to auth libraries (e.g., only pass expected header values), and centralize header/cookie parsing to a utility that normalizes and validates values.

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

# Issue Risk Level
1 Missing input validation for email, organizationId, and roles HIGH
2 Creates user accounts without requiring email verification HIGH
3 Admins can assign 'owner' role, enabling privilege escalation HIGH
4 User enumeration possible via differing responses/side effects HIGH
5 Race condition creating users may lead to duplicate accounts HIGH
6 Storing roles as CSV without validation may corrupt role data HIGH

Recommendations:

  1. Validate and sanitize all inputs server-side (email, organizationId, roles). Use a schema validator (e.g., zod) to enforce formats, allowed role values, and maximum lengths before any DB or auth operations.
  2. Do not activate or add unverified users to organizations automatically. Use an invite flow or require email verification before creating/associating active membership. If creating a placeholder user record, ensure it cannot be used for privileged actions until verified.
  3. Enforce strict server-side authorization for role assignment. Disallow assigning 'owner' (and other high-privilege roles) except by existing owners or via a separate restricted workflow. Validate that roles provided are within an allowed set.
  4. Avoid leaking existence information via different side effects or error messages. Normalize responses for both existing and non-existing users (e.g., always return a generic success response for the API call and perform membership changes asynchronously or return a status token). Log detailed info server-side only.
  5. Prevent race conditions by using DB-level unique constraints plus transactions/upserts. Use a single transactional upsert or SELECT ... FOR UPDATE pattern when creating/finding users and members, and handle unique-constraint errors explicitly instead of letting them surface uncaught.
  6. Store roles in a normalized, validated form (e.g., dedicated join table or JSON/array column with enum validation) rather than ad-hoc CSV. Ensure consistent format for creating and updating roles; validate the role values against an allowed enum.
  7. Add explicit error handling for expected DB constraint errors (e.g., unique violations) and return consistent application-level errors. Instrument and monitor these flows to detect frequent race-condition failures.
  8. If auth.api.addMember is an internal RPC, validate inputs again before calling it and ensure it enforces the same role restrictions and verification requirements.

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

# Issue Risk Level
1 Member update lacks organization constraint MEDIUM
2 Deletes all user sessions globally MEDIUM
3 Role checks use includes(), may match substrings MEDIUM
4 No check to prevent removing last/only admin MEDIUM
5 Owner email not validated before sending MEDIUM

Recommendations:

  1. Include organizationId in the member.update where clause (e.g., where: { id: memberId, organizationId: ctx.session.activeOrganizationId }) to avoid accidental cross-organization updates and reduce race-condition risk.
  2. Limit session deletion to the organization context instead of deleting all sessions for the user. If sessions are global, consider revoking only org-scoped tokens or keeping a mapping of sessions to orgs, or store org-scoped tokens and delete those. At minimum, document intended behavior and consider notifying the user if global sessions will be invalidated.
  3. Avoid substring role checks. Use strict equality or a role enum/typed field (e.g., role === 'admin' or an array of roles and check array membership) so 'superadmin' or 'co-owner' can't be misinterpreted.
  4. Prevent removing the last admin/owner: before deactivating the member, count active members with admin/owner roles in the organization (excluding the target). If count would drop to zero, block the removal or require transferring ownership/adding another admin first.
  5. Validate the recipient email before sending notifications (ensure owner.user.email exists and matches a valid email pattern) and add error handling around sendUnassignedItemsNotificationEmail so a missing/invalid email doesn't throw or leak data. Consider logging the absence of a valid owner email and falling back to notifying other admins.

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

# Issue Risk Level
1 Role substring check (includes) can be bypassed (e.g., 'not-admin') HIGH
2 DELETE uses only id; missing organizationId condition (TOCTOU risk) HIGH
3 Detailed errors reveal invitation existence (resource enumeration) HIGH
4 invitationId validated only as string; no format/UUID check HIGH
5 console.error may log sensitive info from caught errors HIGH

Recommendations:

  1. Fix role checks: avoid substring matching. Use exact comparisons or an enum/whitelist of allowed roles. Example approaches:
  • Store role as a canonical value and compare equality (currentUserMember.role === 'admin' || currentUserMember.role === 'owner').
  • If you support multiple roles in an array, ensure role is parsed into an array and check array.includes('admin') (exact match), not substring matching.
  • Use TypeScript enums or a union type for roles to ensure only known values are possible.
  1. Make the delete operation conditional on organizationId (and status) to eliminate TOCTOU and accidental cross-org deletes. Prefer a single conditional delete and verify affected rows. Examples:
  • Use deleteMany (or delete with a composite PK) with where: { id: invitationId, organizationId: ctx.session.activeOrganizationId, status: 'pending' } and check the returned count.
  • Or wrap the read+delete in a DB transaction and delete by both id and organizationId and confirm 1 row was deleted before returning success.
  1. Avoid resource enumeration via distinct error messages. Return generic error responses for operations that reveal resource existence (e.g., "Unable to revoke invitation"). Log specific reasons server-side only for diagnostics tied to secure logs/monitoring.
  2. Tighten input validation on invitationId. If invitation IDs are UUIDs, use z.string().uuid() in the Zod schema. If other formats are used, validate against that canonical format to reject malformed input early.
  3. Reduce sensitive logging: don't console.error raw errors to stdout/stderr in production. Log a minimal sanitized message and propagate full error details to secure structured logging/monitoring (Sentry, Datadog) behind access controls.
  4. Additional hardening: ensure ctx.session.activeOrganizationId and ctx.user.id cannot be manipulated by clients and are derived only from server-side session state. Consider adding tests for role parsing and race conditions around invite revocation.

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

# Issue Risk Level
1 Exposed server actions may lack permission checks MEDIUM
2 UI uses canManageMembers for client-side access control only MEDIUM
3 organizationId not validated before use in DB queries MEDIUM
4 Role parsing is case-sensitive and may mis-classify roles MEDIUM
5 Full user objects returned to client may expose PII MEDIUM

Recommendations:

  1. Ensure all server actions (including removeMember and revokeInvitation) enforce authorization server-side and cannot be invoked without proper context. Audit authActionClient (safe-action) to confirm it enforces session/context and that every action validates the caller's membership and roles before performing changes.
  2. Do not rely on canManageMembers only for UI gating. Always enforce permission checks on every server action invoked from the client and treat client-side flags as UX-only.
  3. Validate and canonicalize organizationId before using it in DB queries. Also ensure the current session user is an active member of the organization before performing any org-scoped queries or mutations (fail closed if currentUserMember is missing).
  4. Normalize and strictly compare roles on the server (e.g., toLowerCase() and compare to explicit role constants). Avoid substring checks (e.g., role.includes('admin')) without normalization and explicit matching to prevent misclassification.
  5. Return only the minimal user fields required by the client (e.g., id, name, avatar) or explicitly redact PII (email, phone) before sending to the client. If full user objects are necessary, document and justify why, and ensure consent and data handling policies are followed.

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

# Issue Risk Level
1 Client-side owner-check only; bypassable by crafted requests HIGH
2 Client can call authClient.updateMemberRole — risk of role escalation HIGH
3 Mutations (remove/revoke) lack try/catch; errors may leak info HIGH
4 Search query from URL used unsanitized — possible XSS in other contexts HIGH
5 No visible CSRF protection for client-triggered mutations HIGH

Recommendations:

  1. Enforce all authorization and owner-role invariants on the server. Never rely on client-side checks to prevent privilege changes.
  2. Remove direct role-changing calls from client code. Expose server-side endpoints or server actions that perform validation/authorization before changing roles. If using an SDK, only call it from server-side code where secrets/privileges can be enforced.
  3. Wrap all async mutation calls in try/catch and normalize error responses from the server to avoid exposing raw internal errors. Show user-friendly messages and log details server-side.
  4. Treat URL-derived inputs as untrusted. Validate and canonicalize query values server-side and sanitize any place user input is rendered into the DOM. Although React escapes content by default, avoid inserting user-controlled strings into innerHTML or other unsafe sinks.
  5. Ensure CSRF protections for state-changing endpoints: use SameSite=strict/lax cookies, anti-CSRF tokens for cookie-authenticated endpoints, and validate origin/host headers where applicable. Prefer server actions or authenticated API endpoints that include CSRF defenses.

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

# Issue Risk Level
1 Missing validation of fleetDmLabelId used in fleet API path MEDIUM
2 No error handling for fleet.get requests; one fail rejects all MEDIUM
3 Unbounded Promise.all on many requests can cause DoS/resource exhaustion MEDIUM
4 Host IDs are not deduplicated, causing redundant API calls MEDIUM
5 Relies on session.activeOrganizationId without explicit authorization checks MEDIUM

Recommendations:

  1. Validate/sanitize fleetDmLabelId before interpolating into API paths (e.g., ensure it matches expected numeric/UUID format, or parseInt and verify > 0). Avoid directly concatenating unvalidated values into request paths.
  2. Handle individual fleet.get failures: use Promise.allSettled or per-request try/catch so a single downstream error doesn't reject the whole batch. Log and surface partial results appropriately.
  3. Limit concurrency when making many downstream requests (e.g., batch requests, use p-limit, or a queue) to avoid saturating resources and triggering DoS/resource exhaustion.
  4. Deduplicate host IDs (e.g., use a Set) before fetching hosts to avoid redundant API calls and reduce load.
  5. Explicitly assert session integrity and authorization server-side: verify the session user is a member of the organization (or has the expected role/permissions) before returning organization-scoped data. Consider an additional database check against the membership table or a scoped token check.

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

# Issue Risk Level
1 Unvalidated orgId used directly in DB query MEDIUM
2 Fetching all member records may expose excessive data MEDIUM
3 No explicit authorization check beyond session orgId MEDIUM
4 Assumes member.role is non-null string (may throw) MEDIUM

Recommendations:

  1. Validate and normalize orgId before using it in DB queries (e.g., assert type/format: UUID or integer). Prefer verifying organization membership/permission via an authoritative server-side check rather than trusting a value derived from the session payload alone.
  2. Select only required member fields in the DB query (use select/projection) instead of returning all columns. Also push filtering into the DB where possible (e.g., roles filter) to avoid fetching full rows.
  3. Add explicit authorization checks: confirm the authenticated user has permission to list members for orgId (e.g., query the user's membership/role in the organization and enforce RBAC/ACL rules).
  4. Handle null/invalid member.role safely before calling string methods. Example: const roleStr = (member.role ?? ''); const roles = roleStr.includes(',') ? roleStr.split(',') : (roleStr ? [roleStr] : []);
  5. Implement pagination/limits (take/skip or cursor) for findMany to avoid loading large result sets into memory. Add sensible defaults and/or server-side caps.
  6. For performance and safety, perform role-based filtering in the DB query (where: { OR: [{ role: 'employee' }, { role: 'contractor' }] }) rather than filtering allMembers client-side.
  7. Ensure the DB layer/ORM is used in a parameterized way (most ORMs do this by default). If you accept any user-controllable strings into queries elsewhere, sanitize or use parameterized queries to prevent injection.

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

# Issue Risk Level
1 Rendered unvalidated log fields (description/changes) — potential XSS MEDIUM
2 Avatar URL used directly in image src — unsafe protocols possible MEDIUM
3 No validation of timestamps or IDs — may crash or be abused (DoS) MEDIUM
4 console.error may leak raw log data in production MEDIUM

Recommendations:

  1. Sanitize and validate any user-/external-derived strings before rendering. Although React escapes text nodes by default, avoid inserting HTML or using dangerouslySetInnerHTML. For fields that may contain rich HTML (if any), sanitize with a well-maintained sanitizer (e.g., DOMPurify) on the server or before render.
  2. Treat avatarUrl as untrusted. Validate the URL using the URL constructor and restrict acceptable schemes (prefer https:). Reject javascript: and other unsafe protocols. If you must support data: URLs, explicitly validate that they are safe (e.g., only allow base64-encoded PNG/JPEG). Consider proxying avatar images through a safe image service or using CSP to mitigate SVG/script execution risks.
  3. Defensively validate timestamps before calling date-fns.format. Accept only Date objects or numbers (epoch ms) and guard with isValid(date) (date-fns) or a try/catch around format. Provide fallbacks (e.g., 'Unknown date') when invalid. Likewise, guard substring calls (userId/entityId) with existence/length checks to avoid runtime exceptions.
  4. Avoid logging raw production data to console. Remove or gate console.error with environment checks (only log in non-production) and ensure redaction of any sensitive fields if logging is required. Consider a structured logger that can redact or filter sensitive data.
  5. Add schema validation for logs (e.g., zod/joi) at the boundary where logs are ingested or before rendering. Validate shape of log.data (action, details, changes), types of fields, and constrain allowed values for enums (action/entityType).
  6. When rendering change values that may be objects, convert safely (e.g., JSON.stringify with replacer/size limits) and avoid direct toString() on arbitrary objects. Limit depth/size to prevent UI DoS from huge payloads.

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

# Issue Risk Level
1 Unvalidated entity IDs in DB queries (IDOR risk) across endpoints HIGH
2 Missing role/permission checks before returning org data HIGH
3 Exposes PII (user emails/names/images) in responses HIGH
4 Audit logs and member lists may be leaked to unauthorized users HIGH
5 Direct trust of session.activeOrganizationId without extra verification HIGH
6 No input validation/sanitization for policyId and other params HIGH

Recommendations:

  1. Enforce authorization checks on every endpoint: verify that the calling user is a member of the organization (db.member lookup) and has the required role/permissions before returning data (e.g., require admin/auditor roles for audit logs and member lists).
  2. Do not rely solely on session.activeOrganizationId. Verify it against a db.member entry for the current user (e.g., findUnique where userId = session.userId and organizationId = session.activeOrganizationId and deactivated = false).
  3. Harden ID handling: validate and canonicalize incoming IDs (e.g., ensure policyId is a UUID) and fail fast on malformed IDs.
  4. Minimize returned fields and redact PII: use explicit select/projection instead of include to return only fields the caller is authorized to see. For example, avoid returning email/name/image unless the caller has explicit permission.
  5. Add fine-grained RBAC/ABAC checks for sensitive operations: limit access to audit logs, member lists, and attachments to authorized roles, and consider separate scopes for read/write.
  6. Add input validation and centralized sanitization for route params (policyId, commentId, etc.) and add robust error handling for invalid or missing organization membership.
  7. Limit audit log exposure: return truncated summaries where possible, paginate/limit results (already uses take:3 for logs but apply role checks), and avoid including related user/member details unless necessary and authorized.
  8. Log suspicious access attempts (attempts to access org resources without membership) and return generic error messages to callers.

🟡 apps/app/src/app/(app)/[orgId]/risk/(overview)/page.tsx (MEDIUM Risk)

# Issue Risk Level
1 orgId used directly in DB query without validation MEDIUM

Recommendations:

  1. Enforce authorization before using params.orgId in any DB query. Compare params.orgId to the authenticated session's activeOrganizationId or verify the session user is a member of params.orgId before querying db.onboarding.
  2. For the onboarding lookup (db.onboarding.findFirst({ where: { organizationId: orgId } })), require a valid session and membership check and return 401/403 if the user lacks access. Do not reveal organization-specific metadata to unauthorized users.
  3. Normalize scoping: either always use session.session.activeOrganizationId for all queries in this page or explicitly validate that params.orgId matches the session's activeOrganizationId. Avoid mixing param-based orgId with session-based data.
  4. Add defensive validation/sanitization of params.orgId (e.g., ensure it is the expected UUID format) and centralize org authorization checks in a helper to reduce the chance of missing checks in future DB queries.
  5. Audit other DB queries in this route (and adjacent routes) to ensure no other queries use params without authorization checks.

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

# Issue Risk Level
1 Server-side cache can leak org/session data across requests (getAssignees/getRisk) HIGH
2 getRisk cached by riskId only; session ignored -> possible auth bypass/data leak HIGH
3 Unvalidated route params used in DB queries (riskId) — missing validation HIGH
4 Possible stored XSS from DB fields (risk.title/comments) if not sanitized HIGH

Recommendations:

  1. Remove or redesign caching for functions that depend on per-request session/org state. Do not use react/cache without incorporating per-request/session context.
  2. If caching is required, include session-scoped values (e.g., activeOrganizationId) or caller identity in the cache key so cached results cannot be served across different orgs/sessions.
  3. Validate and canonicalize route parameters (riskId) server-side (e.g., enforce UUID format or expected ID schema) before using them in DB queries and return 400 for invalid formats.
  4. Perform explicit per-request authorization checks and avoid returning cached records before verifying the current session has access to the requested org/resource.
  5. Ensure all user-controlled content (risk.title, comments, other DB fields) is escaped or sanitized before rendering. Review Comments and RiskOverview components and remove any use of dangerouslySetInnerHTML or HTML injection. Use a safe rendering library or whitelist-based sanitizer for stored HTML if rich content is required.

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

# Issue Risk Level
1 Full user object fetched (user: true) may expose sensitive fields MEDIUM
2 Unbounded findMany queries can return large datasets or enable DoS MEDIUM
3 Trusting session.activeOrganizationId without additional authorization checks MEDIUM

Recommendations:

  1. Select only the user fields you need instead of using user: true (e.g., select id, email, name). Avoid returning sensitive fields such as hashedPassword, reset tokens, or PII unless strictly required and authorized.
  2. Add pagination/limits to all findMany calls (use take/skip or cursor-based pagination) and enforce server-side maximums/caps to avoid very large responses. Consider adding a hard cap and/or require explicit pagination parameters.
  3. Limit selected fields across all queries (tasks, controls, evidenceAutomations) to only what's rendered by the UI to reduce data exposure.
  4. Enforce explicit authorization checks in addition to trusting session.activeOrganizationId: verify the session user actually has permission to access the requested org and the requested resources (RBAC checks). Log and fail closed if authorization cannot be established.
  5. Add rate limiting / query cost controls (and/or background jobs for heavy exports) to reduce the risk of DoS from expensive queries.

💡 Recommendations

View 3 recommendation(s)
  1. Remediate the OSV CVEs: remove/upgrade xlsx@0.18.5 (GHSA-4r6h-8v6p-xvw6, GHSA-5pgg-2g8v-p4x9) to a patched release and upgrade ai to >= 5.0.52 (per scan).
  2. Validate and canonicalize all external IDs and route params before DB use (e.g., zod schemas like z.string().uuid() or strict regex). Always include organizationId in DB WHERE clauses and avoid spreading updateData — build an allowlisted update object to prevent mass-assignment/IDOR.
  3. Stop trusting client-supplied approverId and header-derived paths: derive approver from the authenticated session (e.g., ctx.user.id) and whitelist/validate revalidation paths; sanitize inputs passed to downstream libraries (including values passed into xlsx) to reduce injection/DoS surface.

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

@chasprowebdev chasprowebdev changed the title [dev] [chasprowebdev] chas/deactivate-member Make the member deactivated when removing it from the org Nov 20, 2025
@chasprowebdev chasprowebdev marked this pull request as ready for review November 20, 2025 02:13
@chasprowebdev
Copy link
Contributor

  1. add deactivated field to Member table
  2. update the API for removing a member from the org
  3. show "deactivated" tag on app (Comments, RecentAuditLogs)
  4. show only activated members on app
  5. disable deactivated member to access to portal
  6. if the removed user was assigned as an approver/assignee for policy, risk, vendor, and task, reset these fields and email an org admin and tell them that there is no longer an assignee.
  7. should be able to invite the deactivated users in the future.

@chasprowebdev
Copy link
Contributor

image image

@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