Skip to content

Conversation

@github-actions
Copy link
Contributor

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

@comp-ai-code-review
Copy link

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

🔒 Comp AI - Security Review

🔴 Risk Level: HIGH

No OSV/NPM CVEs detected. Scan shows code-level auth/injection issues: IDOR via X-Organization-Id, unsanitized domain inputs used in URLs/queries, and unsafe use of request-derived values in DB/commands.


📦 Dependency Vulnerabilities

✅ No known vulnerabilities detected in dependencies.


🛡️ Code Security Analysis

View 7 file(s) with issues

🟡 apps/api/src/main.ts (MEDIUM Risk)

# Issue Risk Level
1 CORS allows any origin with credentials enabled MEDIUM
2 Swagger UI enabled in all environments MEDIUM
3 Swagger persistAuthorization stores credentials in browser MEDIUM
4 OpenAPI JSON written to repo path in non-production (info leak) MEDIUM
5 No rate limiting or security headers configured MEDIUM

Recommendations:

  1. CORS: Do not allow any origin while allowing credentials. Replace origin: true with a specific allowlist or an origin validation function that only echoes back known good origins. If cookies/auth must be used cross-origin, ensure the exact origin is listed and validated (e.g., origin: process.env.ALLOWED_ORIGINS or function that checks against a list).
  2. Swagger UI: Restrict Swagger to non-production environments or protect the UI with authentication/authorization and network restrictions. Wrap SwaggerModule.setup behind an environment check or middleware that enforces IP allowlist / auth.
  3. Swagger persistAuthorization: Set swaggerOptions.persistAuthorization to false (or remove it) so credentials/tokens are not persisted in the browser storage. Prefer ephemeral auth or require manual re-entry.
  4. OpenAPI JSON write: Stop writing generated OpenAPI JSON into the repository working tree. If documentation artifacts are needed, write them to a secure build artifact storage (CI/CD artifact, S3 with restricted access) or only generate them locally for dev but ensure they are not committed or exposed. At minimum, ensure this is disabled except in safe dev-only contexts.
  5. Rate limiting & security headers: Add rate limiting (e.g., express-rate-limit or NestJS throttler) to protect endpoints from abuse. Add security headers via helmet (CSP, HSTS, X-Frame-Options, X-Content-Type-Options, Referrer-Policy) and review CORS/headers for least privilege.
  6. General: Audit production config to ensure no debug/endpoints serve sensitive data. Log and monitor access to Swagger/OpenAPI if it must remain accessible, and rotate any credentials that were exposed accidentally.

🟡 apps/api/src/trust-portal/dto/domain-status.dto.ts (MEDIUM Risk)

# Issue Risk Level
1 Missing validation on DomainVerificationDto fields (type, domain, value, reason) MEDIUM
2 DomainVerificationDto.type accepts arbitrary values (no enum/IsIn) MEDIUM
3 GetDomainStatusDto regex restricts TLDs to 2-6 chars, rejects valid TLDs MEDIUM
4 Domain regex uses inconsistent case rules and may reject IDNs/punycode MEDIUM

Recommendations:

  1. Add class-validator decorators to DomainVerificationDto fields (e.g., @IsString(), @isnotempty(), @IsOptional() where appropriate) and ensure controllers validate incoming DTOs (use ValidationPipe).
  2. Restrict allowed verification types: define an enum or use @isin(['TXT','CNAME','MX','A','AAAA', ...]) or @IsEnum(YourEnum) for DomainVerificationDto.type so callers cannot pass arbitrary values.
  3. If you ever accept arrays of nested DTOs from clients, validate them with @ValidateNested({ each: true }) and @type(() => DomainVerificationDto) on the parent property so nested objects are validated.
  4. Replace the brittle custom regex with a robust domain validator (for example validator.js isFQDN with options) or update the regex to allow modern TLDs (>6 chars). Normalize IDNs to punycode (punycode.toASCII) before validation to handle internationalized domains.
  5. Normalize domain inputs (toLowerCase/trim) and perform server-side validation before any DB/command use. Always use parameterized queries and avoid passing raw user input to shell/exec or query strings.

🟡 apps/api/src/trust-portal/trust-portal.controller.ts (MEDIUM Risk)

# Issue Risk Level
1 Query DTO passed directly to service (unsanitized user input) MEDIUM
2 Potential SQL/command injection if service uses dto in queries/exec MEDIUM
3 X-Organization-Id header is optional; may allow cross-org access MEDIUM
4 No explicit validation pipe shown; DTO validation not guaranteed MEDIUM

Recommendations:

  1. Validate and sanitize GetDomainStatusDto with ValidationPipe and class-validator
  2. Enforce org scoping: require X-Organization-Id or verify user's org access
  3. Sanitize/escape dto before DB/command use; use parameterized queries
  4. Review trustPortalService for unsafe DB/exec/API usage
  5. Add logging, rate limiting, and monitoring for this endpoint

🟡 apps/api/src/trust-portal/trust-portal.service.ts (MEDIUM Risk)

# Issue Risk Level
1 Missing input validation for 'domain' used directly in API path MEDIUM
2 Silent use of empty VERCEL_ACCESS_TOKEN leading to unauthenticated calls MEDIUM
3 Detailed logs (error.stack/response.data) can leak secrets MEDIUM

Recommendations:

  1. Validate and sanitize the domain input before using it in the URL path: implement an allowlist/regex (e.g., RFC 1035 hostname rules) and call encodeURIComponent(domain) when interpolating into the path. Reject or return BadRequest for invalid domains.
  2. Fail fast if VERCEL_ACCESS_TOKEN is missing: throw an exception at startup (or in the constructor) rather than proceeding with an empty Authorization header. Validate TRUST_PORTAL_PROJECT_ID and VERCEL_TEAM_ID at startup as well.
  3. Avoid logging full error stacks and raw response bodies. Redact or omit sensitive fields. Log only low-sensitivity metadata (HTTP status, error code, short sanitized message) and use correlation IDs to trace issues.
  4. When logging axios errors, avoid printing response.data directly. If you must capture response details for diagnostics, store them in a secure, access-controlled sink after redaction.
  5. Add unit/integration tests and runtime checks to ensure the allowed domain format and required env vars are enforced; consider CI gating to prevent deployments with missing tokens/IDs.

🟡 apps/app/src/app/(app)/[orgId]/settings/trust-portal/components/TrustPortalDomain.tsx (MEDIUM Risk)

# Issue Risk Level
1 Relies on client-side validation only (no server-side validation shown) MEDIUM
2 localStorage flags control DNS verification state and can be tampered MEDIUM
3 checkDnsRecord onSuccess sets localStorage on error (logic inversion) MEDIUM
4 User-supplied domain sent to server actions without shown sanitization MEDIUM

Recommendations:

  1. Enforce server-side validation and sanitization for domain input
  2. Do not trust localStorage for verification; use server-side verified flags
  3. Fix onSuccess logic to set/remove localStorage consistently with real verification
  4. Authorize server actions with server-side checks before trusting client state
  5. Log and rate-limit verification endpoints to prevent abuse

🟡 apps/app/src/hooks/use-domain.ts (MEDIUM Risk)

# Issue Risk Level
1 Unvalidated domain used raw in query param enabling server-side injection/header issues MEDIUM

Recommendations:

  1. URL-encode the domain when building the URL: use encodeURIComponent(domain) or URLSearchParams/URL to construct query strings instead of string concatenation.
  2. Validate the domain on the client with a strict domain regex or allowlist (e.g., permit only letters, digits, hyphens and dots, enforce TLD rules) before sending.
  3. Enforce strict server-side validation and canonicalization of the domain parameter regardless of client validation.
  4. Reject or strip control characters (CR, LF), null bytes, and traversal-like characters from the domain input to prevent header injection or unexpected parsing.
  5. Log, rate-limit, and monitor requests to this endpoint to detect abuse or probing attempts.

🔴 packages/docs/openapi.json (HIGH Risk)

# Issue Risk Level
1 IDOR via X-Organization-Id header allowing access to other organizations HIGH
2 Auth bypass if X-Organization-Id is trusted without tying to session cookie HIGH
3 Spec lists only apikey security; session auth not defined/enforced HIGH
4 Possible SQL injection via unsanitized X-Organization-Id header HIGH
5 Possible SQL injection via unsanitized PATCH body fields HIGH
6 Possible command injection via unsanitized request inputs HIGH
7 Metadata declared as string allowing raw JSON; may enable injection/parsing issues HIGH
8 Missing URL validation for logo/website fields HIGH

Recommendations:

  1. Enforce server-side organization authorization for every request. Never trust X-Organization-Id alone — validate it against the authenticated session (cookie/session store) or API key owner and reject/ignore mismatches.
  2. Define and document session-based authentication in the OpenAPI components.securitySchemes (e.g., cookieAuth) and require appropriate security for endpoints that support sessions. Ensure the runtime enforces both cookie session and organization ownership checks.
  3. Treat all headers and body fields as untrusted input. Validate and normalize X-Organization-Id, path params, query params, headers and JSON bodies using a strict schema/validator before use.
  4. Use parameterized queries / prepared statements or the application's ORM for all database access. Do not interpolate headers/params/body directly into SQL strings.
  5. Audit all places where request data can reach OS commands, shell scripts or exec()-style APIs. Remove direct shell interpolation or sanitize/whitelist values; prefer library APIs rather than spawn/exec when possible.
  6. For fields intended to be JSON (e.g., metadata), change the schema to a proper JSON object type or require validated JSON string parsing server-side with explicit schema validation. Reject unexpected properties or formats.
  7. Add explicit URL format validation for logo, website and any other fields expected to be URLs (use format: uri in OpenAPI and enforce server-side checks). Enforce allowed schemes (https) and length/suffix constraints as appropriate.
  8. Harden file-related endpoints: enforce content-type checks, size limits, file-type whitelists, and use safe temporary storage. When rendering or embedding user-provided data into downloadable scripts (Windows setup scripts, batch files, etc.), strictly escape/whitelist values or avoid injecting user-provided strings into shell scripts.
  9. Ensure signed URLs for attachments expire and are scoped to the requesting organization/user. Validate ownership before generating/returning signed URLs.
  10. Add centralized input sanitization and logging of validation failures. Add automated tests (fuzzing/parameterized tests) for IDOR/auth bypass and injection vectors.

💡 Recommendations

View 3 recommendation(s)
  1. Enforce server-side organization authorization and stop trusting X-Organization-Id alone: validate/normalize the header, compare it to the authenticated session/org, and always use parameterized queries or ORM-bound parameters instead of interpolating headers into SQL or commands.
  2. Add strict input validation and encoding for domain-related DTOs and request params: apply class-validator decorators + Nest ValidationPipe, normalize domains (trim, toLowerCase, punycode if needed), and use encodeURIComponent(domain) (or URL/URLSearchParams) before inserting into URL paths or query strings.
  3. Fail fast and reduce sensitive logging for external API calls: throw an error at startup or service init if required tokens/IDs (e.g., VERCEL_ACCESS_TOKEN) are missing, and redact or avoid logging full error.stack and response bodies (response.data) to prevent secret leakage.

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

@vercel
Copy link

vercel bot commented Oct 15, 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 9:05pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
portal Skipped Skipped Oct 16, 2025 9:05pm

@chasprowebdev chasprowebdev changed the title [dev] [chasprowebdev] chas/show-domain-status CS-37 [Improvement] - render records on trust portal settings in app for vercel Oct 15, 2025
@linear
Copy link

linear bot commented Oct 15, 2025

@comp-ai-code-review
Copy link

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

🔒 Comp AI - Security Review

🟡 Risk Level: MEDIUM

No OSV CVEs found. Code changes show raw domain input used as localStorage key, raw error objects being logged/displayed, and no server-side authorization/ownership check for domain status.


📦 Dependency Vulnerabilities

✅ No known vulnerabilities detected in dependencies.


🛡️ Code Security Analysis

View 2 file(s) with issues

🟡 apps/app/src/app/(app)/[orgId]/settings/trust-portal/actions/domain-status.ts (MEDIUM Risk)

# Issue Risk Level
1 Insufficient domain validation — only min length check MEDIUM
2 No authorization check that org can access the requested domain MEDIUM
3 console.error logs full error objects (may expose secrets) MEDIUM
4 Env secrets used without presence checks or scoped validation MEDIUM

Recommendations:

  1. Tighten domain validation: replace z.string().min(1) with a hostname-specific validator (e.g. a strict hostname regex or use a URL parser and validate host only). Example Zod: z.string().regex(/^(?:a-z0-9?.)+[a-z]{2,}$/i) or z.string().refine(v => { try { new URL('http://' + v); return true } catch { return false } }). This prevents malformed or malicious input.
  2. Enforce authorization/ownership: verify that the requested domain is actually associated with the activeOrganizationId before returning details. Options: (a) map organization -> Vercel project/team server-side and ensure the org's project/team matches the env.TRUST_PORTAL_PROJECT_ID or teamId; (b) check your DB for domain ownership; (c) use the Vercel API to confirm the domain belongs to the project for that org and deny access if not.
  3. Avoid logging raw error objects: don't console.error(error) directly. Sanitize or redact sensitive fields before logging. Log safe summaries only (error.message, error.code) and keep stack traces/access to secure logs only. Example: console.error({ msg: 'Failed to get domain status', err: String(error).slice(0, 200) }).
  4. Validate presence and scope of env secrets at startup: assert required env vars (VERCEL_ACCESS_TOKEN, TRUST_PORTAL_PROJECT_ID, VERCEL_TEAM_ID) on server initialization and fail fast with a safe error if missing. Also ensure the VERCEL_ACCESS_TOKEN has least privilege (only the scopes needed) and is rotated. Avoid using non-null assertions (!) without runtime checks.
  5. Return safe, non-sensitive errors to callers: do not forward raw provider errors. Map errors to user-friendly messages (e.g., 'Failed to get domain status') while logging only redacted details internally.
  6. Consider rate-limiting or throttling this action if it can be abused to enumerate domains or cause excessive provider API calls.

🟡 apps/app/src/app/(app)/[orgId]/settings/trust-portal/components/TrustPortalDomain.tsx (MEDIUM Risk)

# Issue Risk Level
1 Verification state stored in localStorage (client-trustable). MEDIUM
2 LocalStorage flags can be tampered to spoof domain verification. MEDIUM
3 checkDnsRecord onSuccess marks flags even when data.data.error exists. MEDIUM
4 Only client-side domain validation shown; server-side validation not enforced. MEDIUM
5 Using raw server error strings in toast may leak internal info. MEDIUM
6 localStorage key uses domain directly, enabling key collisions or injection. MEDIUM

Recommendations:

  1. Move authoritative verification state to the server. Do not trust client-side localStorage flags for security/authorization decisions — always query the server for domain verification status before permitting use of the domain.
  2. Treat localStorage flags as UI-only hints. Never rely on them for access control or to mark a domain as verified on the server.
  3. Fix the checkDnsRecord onSuccess logic: it currently treats responses with data.data.error by setting verification flags and persistently writing 'true' to localStorage. Reverse the logic so that only successful verifications set verified flags; error responses should not mark verification as true.
  4. Validate and authorize domain changes on the server side. Ensure updateCustomDomain and domain-status endpoints perform strict validation and ownership checks (DNS verification, token challenge, or provider verification) before accepting or marking a domain as verified.
  5. Sanitize and normalize the domain value before using it as a localStorage key (for UI caching). Use a safe transformation (e.g., encodeURIComponent or a server-side-generated stable ID or a hash like sha256) to avoid key collisions or unexpected characters.
  6. Do not expose raw server error strings to end users. Log detailed errors server-side, but present generic messages to the user (and optionally a safe error code).
  7. Ensure any client-side validation (zod regex) is mirrored and enforced server-side. Client-side regex is useful for UX but is not a substitute for server validation.
  8. If local caching of verification is desirable for UX, add expiry and explicit invalidation when the server reports differing status; document that it’s purely cosmetic and cannot be used for authorization.

💡 Recommendations

View 3 recommendation(s)
  1. Validate and normalize domain inputs server-side before any use: use a strict hostname regex or URL parser and reject malformed values. When persisting a client-side cache key, encode or hash the domain (e.g., encodeURIComponent or sha256) to avoid key injection/collisions.
  2. Stop logging/returning raw error objects or provider error strings. Log only sanitized details (error.message, error.code) and return a generic error message to the client to avoid leaking sensitive data.
  3. Enforce server-side authorization/ownership checks in the domain-status and domain update endpoints: verify the domain belongs to the activeOrganizationId (DB check or provider API confirmation) before returning status or marking a domain verified.

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

@vercel vercel bot temporarily deployed to Preview – portal October 15, 2025 03:04 Inactive
@chasprowebdev chasprowebdev requested a review from Marfuen October 15, 2025 03:05
@chasprowebdev
Copy link
Contributor

image

@Marfuen Marfuen merged commit 914c45e into main Oct 16, 2025
12 checks passed
@Marfuen Marfuen deleted the chas/show-domain-status branch October 16, 2025 21:28
@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.

4 participants