Skip to content

Conversation

@MiViewIT
Copy link

@MiViewIT MiViewIT commented Oct 3, 2025

What does this PR do?

A few quality of life changes are included. As a do-er the flow is pretty straight forward, but as someone setting up the system its not clear how to explore and drill into the underlying frameworks and compliance policies that make them up. So it makes this software hard to see how it would be used in an organization that has already made any amount of effort to be compliant.

When the user enters advanced mode, a new "View Details" section appears on the frameworks widget. This is to help better navigate what a Framework is made up of.

There was already a "+ Add Control" button and ui, i added that button to each of the control item screens.

When creating a control, a user may want to create a task a new "+ Create New Task" button was implemented, it changes the action button to "Create $ Continue" which will complete the normal control creation flow, then open the Task creation flow and prepopulate the control that was just created.

Image Demo:

image Screenshot 2025-10-03 at 3 38 36 PM Screenshot 2025-10-03 at 3 39 03 PM Screenshot 2025-10-03 at 3 56 28 PM

Mandatory Tasks (DO NOT REMOVE)

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

How should this be tested?

We piggyback on exsisting framework, no new tests needed. We could make one to ensure the handoff from control to task dosnt break.

Checklist

I developed and tested on the local branch code PR#1555, this area cherry picks to make sure that branch can be moved along, i believe these to be the standalone changes required for implementation. Please Test.

Also the "View Details" button should be moved to look more cohesive. i figured you guys would want to massage it here.

enabling advanced mode allows the ui to show a see more button on each framework, so users can see the details of them in a natural progression kind of way.
placed the add control button to the requirements page. this way if there are missing controls they can be added manually in natural progression.
added task creation to the control creation flow
updated readme to include DB connection pooling notes, so trigger.dev dosnt overwhelm smaller db's
merge comments got left in somehow, weird. removed them.
@vercel
Copy link

vercel bot commented Oct 3, 2025

@james-miview is attempting to deploy a commit to the Comp AI Team on Vercel.

A member of the Team first needs to authorize it.

@MiViewIT MiViewIT marked this pull request as ready for review October 3, 2025 20:55
@comp-ai-code-review
Copy link

comp-ai-code-review bot commented Oct 16, 2025

🔒 Comp AI - Security Review

🔴 Risk Level: HIGH

No OSV CVEs found. Scanner flags input-validation and injection risks: raw client inputs sent to server (CreateControlSheet/CreateTaskSheet) and unsanitized route/search params used in DB queries (controls/page.tsx, frameworks/page.tsx).


📦 Dependency Vulnerabilities

✅ No known vulnerabilities detected in dependencies.


🛡️ Code Security Analysis

View 5 file(s) with issues

🟡 apps/app/src/app/(app)/[orgId]/controls/components/CreateControlSheet.tsx (MEDIUM Risk)

# Issue Risk Level
1 No server-side validation; relies on client zod only MEDIUM
2 User inputs sent to server may enable SQL injection if unsanitized MEDIUM
3 User inputs may enable command injection on server if used in exec MEDIUM
4 Displays raw server error in toast, leaking internal info MEDIUM

Recommendations:

  1. Enforce server-side validation for the create-control endpoint using a schema (e.g., the same zod schema) and reject/normalize invalid input on the server before any processing or DB queries.
  2. Treat client-side validation as UX-only. Never trust client input: validate types, lengths, allowed IDs, and canonicalize values server-side.
  3. Use parameterized queries or an ORM with query parameterization (prepared statements) for all DB access to eliminate SQL injection risks. Do not concatenate user input into SQL strings.
  4. Do not execute shell commands with user-controlled input. If executing external processes is required, strictly validate and whitelist arguments or use safe libraries that avoid shell interpretation.
  5. Avoid returning raw server error strings to the client. Log detailed errors server-side (with correlation IDs) and return sanitized, user-friendly error messages to the client.
  6. Implement input size limits, enumerations/whitelists for IDs, and strict type checks for fields like policyIds, taskIds, requirementMappings, and frameworkInstanceId.
  7. Introduce server-side logging and monitoring for anomalous input patterns and error rates; consider sanitizing/logging sensitive fields.
  8. If the server is in this repository, add tests that assert server rejects invalid/malicious inputs and verify DB queries use parameter binding.

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

# Issue Risk Level
1 getControls invoked before session/orgId is obtained HIGH
2 User-controlled searchParams passed to getControls (unsanitized filters) HIGH
3 include: { user: true } returns all user fields, may leak sensitive data HIGH
4 findMany used without limits/pagination (potential data exposure/DoS) HIGH
5 Session fetched separately in each helper; inconsistent auth checks possible HIGH

Recommendations:

  1. Pass and enforce orgId into getControls and server-side queries
  2. Validate and sanitize searchParams; avoid building raw queries with user input
  3. Select only required user fields instead of include: { user: true }
  4. Call auth.api.getSession once and reuse session per request
  5. Add pagination/limits to findMany queries to prevent large responses

🟡 apps/app/src/app/(app)/[orgId]/frameworks/[frameworkInstanceId]/requirements/[requirementKey]/page.tsx (MEDIUM Risk)

# Issue Risk Level
1 Route params (frameworkInstanceId, requirementKey) not validated MEDIUM
2 Fetching full org datasets (tasks, members) without pagination MEDIUM
3 Members query includes full user object exposing PII MEDIUM

Recommendations:

  1. Validate and sanitize frameworkInstanceId and requirementKey before use (e.g., enforce UUID format or allowed character set and max length). Reject or normalize invalid values early to avoid logic errors and unexpected DB queries.
  2. Add explicit authorization/permission checks beyond session existence (e.g., verify user has access/role for the framework instance). Although getSingleFrameworkInstanceWithControls enforces organizationId matching (see reasoning), consider role-based checks if different members of the org have different privileges (read vs. edit).
  3. Limit dataset sizes and add pagination or result caps for task and member queries (e.g., limit + cursor/offset). Consider on-demand loading for large lists.
  4. Avoid including full user objects when not required. In the members query, select only the fields needed for the UI (e.g., user.id, user.name, user.avatarUrl). Remove or redact PII fields (email, phone, etc.) unless explicitly needed and allowed.
  5. Consider adding server-side rate limiting and request logging for sensitive endpoints to detect abuse and brute-force attempts.
  6. Where applicable, use explicit select clauses for all db.findMany/findUnique calls to minimize accidental data exposure and make access patterns explicit.

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

# Issue Risk Level
1 Missing authorization: session presence not checked against orgId (IDOR risk) HIGH
2 Inconsistent org context: getControlTasks uses session.activeOrganizationId not route orgId HIGH
3 No validation of orgId param before DB queries HIGH
4 No explicit permission checks on sensitive reads (frameworks, tasks, policies) HIGH
5 Potential cross-org data exposure via unconstrained DB queries HIGH

Recommendations:

  1. Enforce membership/permission check: after resolving session and organizationId from params, verify session.user.id is a member of organizationId and has required permissions before performing any DB reads. If the member is missing or lacks permissions, return 403/404 and stop processing.
  2. Use a single authoritative orgId for all queries: either require session.activeOrganizationId to match the route orgId and reject if they differ, or pass the route orgId explicitly into helper functions (e.g., getControlTasks) so they don't read session-scoped org id internally.
  3. Change getControlTasks to accept organizationId as an argument and include that orgId in the cache key (or avoid caching per-session org data). Avoid reading session inside a cached function so cached results cannot leak between org contexts.
  4. Validate/sanitize route orgId early: enforce expected format (e.g., UUID regex) and canonicalize/normalize it before using in DB queries. Reject malformed IDs with 400/404.
  5. Apply least privilege to DB reads: limit selects to only required fields (already done for org query but apply everywhere), and fail closed if the authenticated user is not a member or lacks privileges.
  6. Return explicit authorization errors instead of continuing or silently rendering: use 403 for authenticated but unauthorized, 404 if you don't want to reveal resource existence.
  7. Audit all helper functions (getAllFrameworkInstancesWithControls, getFrameworkWithComplianceScores, getPublishedPoliciesScore, getDoneTasks) to ensure they accept and enforce the same orgId and do not perform unconstrained queries based on session state.
  8. If using ORM/parameterized queries (e.g., Prisma), continue to rely on parameterization to prevent SQL injection, but still validate input shape and enforce authorization checks—do not rely on parameterization alone for access control.

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

# Issue Risk Level
1 Client-side validation only; server-side checks not guaranteed MEDIUM
2 Untrusted form/prefill data passed to createTaskAction unsanitized MEDIUM
3 Server error details may be leaked via toast.error message MEDIUM
4 UI open state controlled by query param may be manipulated via URL MEDIUM

Recommendations:

  1. Enforce the same Zod schema (or stronger) server-side in createTaskAction and reject/validate all incoming fields on the server before any persistence or privileged operations.
  2. Treat all client-supplied and prefilled data as untrusted. Perform server-side sanitization/validation and authorization checks (e.g., ensure assigneeId belongs to the org, department/frequency values are valid enums) before acting on the data.
  3. Do not return raw server error messages to the client. Return generic error messages (e.g., "Failed to create task") and log detailed errors server-side with correlation IDs. Ensure the onError handler doesn't surface stack traces or internal error objects.
  4. Verify authorization on the server for any actions triggered by client UI state (e.g., creating a task for a given assignee/department) — do not rely on client-side UI state or URL params for authorization decisions.
  5. Add server-side input size limits and rate limiting on the task creation endpoint to mitigate abuse. Validate array lengths (controlIds), string lengths, and reject overly large payloads.
  6. If any prefilled/template data is rendered in contexts that may interpret HTML (not the case with standard React inputs, which auto-escape), properly escape or sanitize it to avoid XSS. Prefer React's default escaping and avoid dangerouslySetInnerHTML unless strictly necessary and sanitized.

💡 Recommendations

View 3 recommendation(s)
  1. Implement server-side validation for create endpoints referenced by CreateControlSheet.tsx and CreateTaskSheet.tsx (reuse/replicate the Zod schemas) and reject/normalize invalid input before any processing.
  2. Ensure all DB access in controls/page.tsx and frameworks/page.tsx uses parameterized queries/ORM binding and only accepts validated route/search params (pass a validated orgId/frameworkInstanceId into helpers rather than relying on raw params).
  3. Do not forward raw server error strings to the client from action handlers; return sanitized user-facing errors and log full details server-side (include filename/handler names where logged, e.g., create-control/create-task handlers).

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

@vercel
Copy link

vercel bot commented Oct 16, 2025

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

Project Deployment Preview Comments Updated (UTC)
app Ready Ready Preview Comment Oct 16, 2025 3:01pm
portal Ready Ready Preview Comment Oct 16, 2025 3:01pm

@Marfuen
Copy link
Contributor

Marfuen commented Oct 16, 2025

@MiViewIT can you please review the changes requested from the comp-ai-code-review bot? Thanks

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants