Skip to content

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Oct 6, 2025

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

@vercel
Copy link

vercel bot commented Oct 6, 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 14, 2025 2:38pm
portal Ready Ready Preview Comment Oct 14, 2025 2:38pm

@comp-ai-code-review
Copy link

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

🔒 Comp AI - Security Review

🔴 Risk Level: HIGH

OSV scan: no CVEs. Static analysis found SQL injection risks, missing input validation, and tenant-authorization flaws in API controllers/services.


📦 Dependency Vulnerabilities

✅ No known vulnerabilities detected in dependencies.


🛡️ Code Security Analysis

View 7 file(s) with issues

🔴 ENTERPRISE_API_AUTOMATION_VERSIONING.md (HIGH Risk)

# Issue Risk Level
1 SQL injection via unsanitized automationId in DB query HIGH
2 Missing input validation for orgId/taskId/automationId/version HIGH
3 Authorization checks not enforced - can access other orgs' scripts HIGH
4 S3 key manipulation allows cross-org or arbitrary object access HIGH
5 Race condition when computing next version leads to conflicts/overwrite HIGH
6 Detailed S3/DB errors may leak internal info HIGH

Recommendations:

  1. Input validation & sanitization: apply strict allowlist validation for orgId, taskId, automationId (e.g., regex that matches expected ID formats) and ensure version is an integer. Reject or canonicalize any input that doesn't match.
  2. Use parameterized/ORM queries: never interpolate request-derived IDs into SQL strings. Use parameterized queries or an ORM that binds parameters to prevent SQL injection.
  3. Enforce authentication & RBAC: require authentication and enforce per-organization/task/automation authorization checks on every endpoint. Verify the authenticated principal has explicit access to the requested orgId/taskId/automationId before any S3 or DB operations.
  4. Prevent S3 key manipulation: construct S3 keys server-side from validated IDs only. Normalize and reject suspicious characters. Do not accept full S3 keys or path segments from the client. Use an allowlist of permitted key prefixes if possible.
  5. Make version allocation atomic: obtain the next version number in the database atomically to avoid races (e.g., use a database sequence, a transaction with SELECT ... FOR UPDATE on a version row, or an insert with a unique constraint on (automationId, version) + retry on conflict).
  6. Least privilege for S3 access: ensure the Enterprise API uses IAM credentials with least privilege limited to the specific S3 prefixes needed, and avoid overly broad S3 permissions.
  7. Sanitize and genericize error messages: do not return raw DB/S3 stack traces to clients. Return generic messages (e.g., 'Not found', 'Operation failed') and log detailed errors server-side for debugging.
  8. Concurrency controls & retries: implement optimistic retry/backoff on copy/version-creation failures due to concurrent publishes. Consider a unique constraint and handle duplicate-key errors gracefully.
  9. Audit & logging: log publish/restore events (who performed action, automationId, version, timestamps) and alert on unexpected access patterns.
  10. Testing & fuzzing: add tests for malformed IDs, unauthorized access attempts, concurrent publish requests, and S3 copy failures to validate behavior and error handling.

🟡 apps/api/src/automation/automation.controller.ts (MEDIUM Risk)

# Issue Risk Level
1 No input validation on request bodies MEDIUM
2 No validation or sanitization of automationId param MEDIUM
3 Missing resource-level authorization check MEDIUM
4 Possible SQL/command injection if service uses raw inputs MEDIUM

Recommendations:

  1. Apply ValidationPipe and class-validator decorators on DTOs
  2. Validate and sanitize automationId (e.g., ParseUUIDPipe)
  3. Enforce resource-level authorization checks per user/org
  4. Ensure services use parameterized queries or ORM safe APIs
  5. Log and monitor failed auth and validation events

🔴 apps/api/src/automation/automation.service.ts (HIGH Risk)

# Issue Risk Level
1 Missing input validation for create and update DTOs HIGH
2 Mass assignment: update uses DTO directly allowing unexpected field updates HIGH
3 No authorization checks for create/update operations HIGH

Recommendations:

  1. Enforce DTO validation and sanitization: use class-validator decorators (e.g., IsUUID, IsString, Length) on CreateAutomationDto and UpdateAutomationDto and enable Nest's ValidationPipe (prefer app.useGlobalPipes(new ValidationPipe({ whitelist: true, forbidNonWhitelisted: true }))).
  2. Prevent mass-assignment on update: do not pass the incoming DTO directly to the ORM. Map/whitelist allowed fields explicitly (e.g., const data = { name: dto.name, description: dto.description } ) and pass that object to db.evidenceAutomation.update.
  3. Add authorization checks: verify the authenticated user is allowed to create an automation for the task and is the owner (or has appropriate role) before creating/updating. Inject and use an auth context or pass the user id into the service and check task/automation ownership in the DB queries.
  4. Validate taskId format and existence earlier: use IsUUID on taskId in CreateAutomationDto and validate proactively before DB operations to fail fast and avoid unexpected DB behavior.
  5. Add limits on input size and field lengths in DTOs to mitigate abuse (e.g., maximum lengths for name/description).
  6. Consider using database-level protections (e.g., allowed update columns, RBAC) and logging/auditing for create/update operations.

🔴 apps/api/src/tasks/automations/automations.controller.ts (HIGH Risk)

# Issue Risk Level
1 IDOR: endpoints use automationId without verifying it belongs to task HIGH
2 Missing validation: create/update DTOs not validated in controller HIGH
3 Unvalidated query params: limit/offset parsed without bounds or NaN handling HIGH
4 Unvalidated IDs/params passed directly to services may reach DB/queries HIGH
5 Possible mass-assignment if DTOs include unsafe fields HIGH

Recommendations:

  1. Verify automation belongs to the task before returning/updating/deleting
  2. Use Nest validation pipe (class-validator) for DTOs and route params
  3. Validate numeric queries: enforce min/max and handle NaN
  4. Add authorization checks in service layer (defense-in-depth)
  5. Whitelist updatable fields to prevent mass-assignment

🔴 apps/api/src/tasks/automations/automations.service.ts (HIGH Risk)

# Issue Risk Level
1 Missing org ownership checks on update/delete/findById/listVersions HIGH
2 findByTaskId lacks org verification allowing cross-org access HIGH
3 Mass assignment: update passes DTO directly to db.update HIGH
4 Inputs (IDs, limit, offset) not validated or sanitized before DB use HIGH
5 Object existence can be enumerated across organizations HIGH

Recommendations:

  1. Require organizationId (from auth context) for all operations and include it in DB where clauses, e.g. where: { id: automationId, task: { organizationId } } or join via related task/organization fields to enforce tenant ownership.
  2. For findByTaskId, accept/obtain the caller's organizationId and verify the requested task belongs to that organization before returning automations.
  3. Avoid mass-assignment by mapping the incoming DTO to an allowlist of fields when updating: explicitly pick only name, description, enabled, etc., instead of passing updateAutomationDto directly to db.update.
  4. Validate and sanitize all external inputs: ensure IDs are valid UUIDs (or expected format), and enforce numeric ranges for pagination (limit, offset). Use a validation library (class-validator, zod) or NestJS pipes to enforce types and ranges at the controller boundary.
  5. Return consistent authorization responses to prevent resource enumeration. Check ownership first and return 403 (or a consistent 404) for unauthorized access rather than exposing existence across organizations. Avoid different error messages that reveal whether a resource exists in another org.
  6. Add automated tests for authorization/tenant isolation and unit tests that assert DB queries include organization constraints.
  7. Log authorization failures for audit, but avoid leaking sensitive details in responses.

🟡 apps/api/src/tasks/tasks.controller.ts (MEDIUM Risk)

# Issue Risk Level
1 Missing input validation for route params and request bodies MEDIUM
2 Potential SQL injection via unsanitized taskId/organizationId in services MEDIUM
3 Insecure direct object reference (taskId) if access checks are absent MEDIUM
4 Mass assignment: upload DTO may allow client-controlled entity fields MEDIUM
5 File upload lacks server-side size/type/content validation MEDIUM
6 No explicit malware/virus scanning for uploaded files MEDIUM
7 Optional X-Organization-Id header may allow org spoofing with API keys MEDIUM

Recommendations:

  1. Enable NestJS ValidationPipe and validate DTOs and route params
  2. Use parameterized queries/ORM APIs; avoid string SQL concatenation
  3. Enforce verifyTaskAccess/ownership checks on all task endpoints
  4. Whitelist/sanitize upload DTO fields; reject client-controlled entity ids
  5. Validate file size/type, scan uploads, and store with safe filenames

🔴 apps/api/src/tasks/tasks.service.ts (HIGH Risk)

# Issue Risk Level
1 No validation of organizationId/taskId HIGH
2 Trusts client-provided organizationId enabling auth bypass HIGH
3 Exposes assignee data without field filtering or permission checks HIGH
4 Logs full error objects to console (possible sensitive leakage) HIGH

Recommendations:

  1. Validate and sanitize organizationId and taskId at the boundary (e.g., route layer) using a strict schema (class-validator, Zod). Reject/400 non-UUID or malformed IDs before they reach the service.
  2. Do not trust client-provided organizationId for authorization. Derive the organization from the authenticated user's token/session (e.g., req.user.organizationId) or perform an explicit ownership check against the authenticated user. Enforce this in route guards or in the service (use userId/orgId from auth context, not from client input).
  3. Limit returned assignee fields. Map DB results to DTOs and only include non-PII fields (e.g., id, displayName) or apply permission checks before including assignee. Remove include: { assignee: true } unless authorized, and explicitly select allowed fields instead of returning the full model.
  4. Avoid logging raw error objects to console. Log sanitized messages and a correlation/request ID, and send minimal error details to logs (stack traces to secure log storage only). Replace console.error(...) with structured logging (winston/pino) and scrub PII from logs.
  5. Add automated tests for authorization paths (attempt using a different orgId) and input validation, and add schema-based request validation middleware at the API boundary.

💡 Recommendations

View 3 recommendation(s)
  1. Replace any string-interpolated SQL/DB calls with parameterized queries or ORM-bound parameters (e.g., Prisma/knex prepared binds) in automation/versioning and task services to eliminate SQL injection paths (files flagged: ENTERPRISE_API_AUTOMATION_VERSIONING.md, automation.service.ts, tasks.service.ts).
  2. Enable NestJS validation at the controller boundary and annotate DTOs/route params (ValidationPipe + class-validator / ParseUUIDPipe) so automationId/taskId/orgId and numeric query params are validated and sanitized before reaching services (apply to automation.controller.ts, tasks/automations controllers).
  3. Enforce tenant/ownership checks server-side: derive organizationId from the authenticated context and include it in all DB where clauses (e.g., where: { id: automationId, task: { organizationId: authOrgId } }) instead of trusting client-supplied orgId to prevent IDOR/cross-org access (apply to tasks.service.ts, automations.service.ts).

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

@Marfuen Marfuen merged commit cbff0f9 into main Oct 14, 2025
10 of 11 checks passed
@Marfuen Marfuen deleted the mariano/more-10 branch October 14, 2025 14:38
@claudfuen
Copy link
Contributor

🎉 This PR is included in version 1.56.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants