Conversation
* feat(rbac): extend better-auth permissions with GRC resources - Update permissions.ts to extend defaultStatements from better-auth - Add GRC resources: control, evidence, policy, risk, vendor, task, framework, audit, finding, questionnaire, integration - Add program_manager role with full GRC access but no member management - Update owner/admin roles to extend ownerAc/adminAc from better-auth - Update auditor role with read + export permissions - Keep employee/contractor roles minimal with assignment-based access - Add ROLE_HIERARCHY, RESTRICTED_ROLES, PRIVILEGED_ROLES exports - Add placeholder for dynamicAccessControl in auth.ts (Sprint 2) Part of ENG-138: Complete Permission System Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(rbac): add PermissionGuard and @RequirePermission decorator - Create PermissionGuard that calls better-auth's hasPermission API - Add fallback role-based check when better-auth is unavailable - Create @RequirePermission decorator for route-level permission checks - Create @RequirePermissions decorator for multi-resource permissions - Export GRCResource and GRCAction types for type safety - Add program_manager to Role enum in database schema - Update AuthModule to export PermissionGuard The guard: - Validates permissions via better-auth's hasPermission endpoint - Falls back to role-based check if API unavailable - Logs warnings for API key bypass (TODO: add API key scopes) - Provides static isRestrictedRole() helper for assignment filtering Part of ENG-138: Complete Permission System Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(rbac): sync portal permissions with app permissions - Update portal permissions.ts to match app version - Fix security issue where employee/contractor had excessive permissions - Add program_manager role to portal - Extend defaultStatements from better-auth - Add RESTRICTED_ROLES and PRIVILEGED_ROLES exports BREAKING CHANGE: Employee and contractor roles in portal now have restricted permissions matching the app. Previously they had member management and organization update permissions. Part of ENG-138: Complete Permission System Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test(rbac): add PermissionGuard unit tests Add comprehensive tests for PermissionGuard covering: - Permission bypass when no permissions required - API key bypass behavior - Role-based access for privileged vs restricted roles - Fallback behavior when better-auth API unavailable - isRestrictedRole static method for all role types Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(rbac): migrate controllers from RequireRoles to RequirePermission Migrate all API controllers to use the new better-auth permission system: - findings.controller.ts: finding create/update/delete permissions - task-management.controller.ts: task CRUD + assign permissions - people.controller.ts: member delete permission for removeHost - evidence-export.controller.ts: evidence export permission Also fix TypeScript errors in permission.guard.spec.ts for fetch mocking. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(rbac): add assignment-based filtering for employee/contractor roles Implement assignment filtering to restrict employees/contractors to only see resources they are assigned to: - Add memberId to AuthContext for assignment checking - Create assignment-filter utility with filter builders and access checkers - Update tasks controller/service with assignment filtering on GET endpoints - Update risks controller/service with assignment filtering on GET endpoints - Add PermissionGuard and @RequirePermission to tasks and risks endpoints Employees/contractors now only see: - Tasks where they are the assignee - Risks where they are the assignee Privileged roles (owner, admin, program_manager, auditor) see all resources. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(rbac): add department-based policy visibility Allow admins to control which departments can see specific policies: Schema changes: - Add PolicyVisibility enum (ALL, DEPARTMENT) - Add visibility and visibleToDepartments fields to Policy model API changes: - Add memberDepartment to AuthContext for visibility filtering - Create department-visibility utility with filter builders - Update policies controller to filter by visibility for restricted roles - Update policies service to accept visibility filter Policies can now be: - Visible to ALL (default) - everyone in the organization sees them - Visible to specific DEPARTMENTS only - only members in those departments see them Privileged roles (owner, admin, program_manager, auditor) see all policies regardless of visibility settings. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(auth): centralize auth on API with security hardening - Move auth server to API, app now uses proxy to forward auth requests - Remove localStorage token storage (XSS prevention) - Add rate limiting to auth proxy (60/min general, 10/min sensitive) - Add redirect URL validation to prevent open redirects - Add AUTH_SECRET validation at startup - Make all debug logging conditional on NODE_ENV - Simplify root page routing (no activeOrganizationId dependency) - Use URL-based RBAC with direct DB member lookup Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(rbac): add shared auth package and API integration - Add @comp/auth package with centralized permissions and role definitions - Update API auth module to integrate with better-auth server - Add 403 responses to policy and risk endpoints for Swagger - Add assignment filter and department visibility utilities with tests - Sync permissions across app and portal - Update tsconfig and nest-cli for proper module resolution Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(rbac): enable dynamic access control for custom roles - Add dynamicAccessControl config to organization plugin - Add OrganizationRole table for storing custom roles - Configure maximum 20 roles per organization - Add schema mapping for better-auth role table Resolves: ENG-145 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(rbac): add Custom Roles API for dynamic role management - Add roles module with CRUD endpoints for custom roles - Implement privilege escalation prevention - Add permission validation against valid resources/actions - Protect built-in roles (owner, admin, auditor, employee, contractor) - Add OrganizationRole table migration - Limit to 20 custom roles per organization - Require ac:create/read/update/delete permissions for role management Implements: ENG-146 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(rbac): support multiple roles for privilege escalation checks - Update roles service to accept array of roles instead of single role - Add getCombinedPermissions to merge permissions from all user roles - Update controller to pass full userRoles array - Users with multiple roles now get combined permissions for validation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(auth): prevent JWKS key regeneration causing session loss Add explicit jwks configuration with rotationInterval to prevent better-auth from creating new JWKS keys on each request. Without this, all existing JWTs become invalid when the API restarts because new signing keys are generated. - Set rotationInterval to 30 days for monthly key rotation - Set gracePeriod to 7 days so old keys remain valid after rotation Fixes: Session persistence across API restarts References: - better-auth/better-auth#6215 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test(rbac): add unit tests for Custom Roles API - Add 18 tests for RolesService covering CRUD operations - Add 9 tests for RolesController - Test permission validation and privilege escalation prevention - Test multiple roles support for privilege checking - Test edge cases (duplicate names, max roles limit, reserved names) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: add testing guidelines for API development - Update .cursorrules with testing requirements and conventions - Add apps/api/CLAUDE.md with API-specific development guidelines - Document when to write tests, how to run them, and test patterns - Include RBAC system documentation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor(docs): move API testing rules to apps/api - Remove API-specific testing rules from root .cursorrules - Create apps/api/.cursorrules with API testing requirements - Keep root .cursorrules focused on commit message conventions Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test(rbac): add privilege escalation test for role updates Ensures that users cannot escalate privileges when updating role permissions, not just when creating roles. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(rbac): implement Custom Roles UI (ENG-148) - Add roles settings pages (list, create, edit) with permission matrix - Add "Select all" feature to quickly set all permissions - Integrate custom roles into member management UI: - Role filter dropdown shows all roles dynamically - Invite modal supports custom role selection - Edit member role supports custom roles - Allow normal spelling for role names (spaces, capitalization) - Add loading skeletons with proper PageLayout wrappers - Add comprehensive tests for RolesTable, RoleForm, PermissionMatrix Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore(docs): add API endpoints for managing custom roles * chore(auth): implement cleanup of stale JWKS records on secret change * chore(permissions): implement user permissions resolution and route protection - Add functions to resolve user permissions based on roles and organization - Implement route permission checks to guard access to various app sections - Introduce new layout components for route protection across multiple pages - Update existing components to utilize the new permissions system for access control * chore(api): migrate to session-based authentication and add controls management * refactor(auth): update permission checks to include cookie header * refactor(api): update permission guard logging to include request details * refactor(api): remove unnecessary logging from getPolicy function * chore(api): handle optional chaining for user ID in various actions * refactor(app): update policy editor to check version read-only state * refactor(api): enhance version content handling and validation in policies * refactor(app): update PolicyArchiveSheet to use new design system components * feat(audit): implement audit logging interceptor and related functionality * refactor(audit): add audit log constants, resolvers, and utilities * refactor(auth): add isPlatformAdmin support in authentication and guards * chore(api): add unit tests for PeopleController and PeopleService * chore(tests): add unit tests for layout and questionnaire data queries * chore(api): implement pagination and filtering for risks retrieval * chore(api): add onboarding endpoint to retrieve organization onboarding data * refactor(vendors): migrate vendor data fetching to server API and remove obsolete queries * refactor(api): update controllers and services to use new API structure and enhance data fetching * refactor(soa): remove console logs for component initialization and debugging * refactor(auth): implement service token authentication and update guards * refactor(trust): replace server action with API call for brand settings * chore(openapi): add new endpoints for trust portal settings management * chore(trust): add endpoints for managing trust portal settings and favicon * feat(auth): add API key validation with scopes and new auth controller * refactor(auth): update resource mapping from "portal" to "trust" in permissions * feat(tasks): add bulk submit for review functionality and task approval methods * feat(audit): add comprehensive audit commands for design system, hooks, RBAC, unit tests, and production readiness * feat(auth): add apiKey resource and permissions to roles and decorators * refactor: standardize roles in packages/auth package * refactor(auth): add createdAt field to user response and update environment variables * refactor(env): add internal API token to environment configuration * fix(CompanyFormPageClient): remove orgId parameter from API call * refactor: add AWS credentials validation and integration test actions * refactor(cloud-tests): update authentication method to use session cookie * chore: add unit tests * chore(trust): enhance permission gating tests and mock localStorage * chore(db): add multiple migrations for policy visibility and role management * chore: fix stuff * feat: add audit log controller and integrate with existing modules * feat: add audit log controller and integrate with existing modules * chore: remove CODE_OF_CONDUCT and commitlint configuration files * feat(api): implement pentest billing module with Stripe integration - Add PentestBillingController and PentestBillingService for managing subscriptions and billing. - Implement endpoints for subscription status, creating checkout sessions, handling success callbacks, and managing billing portals. - Integrate role-based access control for billing actions using @RequirePermission. - Introduce tools for AI chat to fetch organization and policy data based on user permissions. - Update app.module.ts to include StripeModule and RolesModule for billing functionalities. - Ensure all new features are covered by tests and adhere to project guidelines. * feat(api): implement triggerEmail function for email notifications - Introduce triggerEmail function to replace sendEmail for sending email notifications. - Update various notifier services to utilize triggerEmail for sending emails. - Add new send-email task to handle email sending via the trigger.dev SDK. - Update package.json and bun.lock to include @react-email/render dependency. - Ensure all changes are covered by tests and adhere to project guidelines. * feat(db): add migrations for policy visibility, organization roles, role notifications, JWKS expiration, and API key scopes - Create new enum type "PolicyVisibility" and update "Policy" table to include visibility and visibleToDepartments fields. - Introduce "organization_role" table for dynamic roles with associated permissions and foreign key constraints. - Add "role_notification_setting" table to manage notification settings per role within organizations. - Extend "jwks" table to include "expiresAt" timestamp for better key management. - Change "permissions" column in "organization_role" from jsonb to text for compatibility with better-auth. * chore: update .gitignore and remove outdated audit findings document - Update .gitignore to reflect the new path for audit findings. - Remove the outdated audit findings document from the repository. * chore(deps): update ai-sdk packages and remove unused DraggableCards component - Upgrade @AI-SDK dependencies to version 3.0.0 for @ai-sdk/anthropic, @ai-sdk/groq, @ai-sdk/openai, @ai-sdk/provider, and @ai-sdk/react. - Update @ai-sdk/rsc to version 2.0.0 and @ai-sdk/provider-utils to version 4.0.19. - Remove the unused DraggableCards component from the project. - Adjust types in InviteMembersModal and other components for better type safety. * fix(api): add error handling to streaming pump in assistant chat controller Wrap the ReadableStream pump loop in try/catch/finally to handle stream read errors gracefully (e.g., client disconnects) and ensure res.end() is always called. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(api): validate organization exists in service token auth Service token auth now verifies the x-organization-id header references a real organization in the database, preventing operations against non-existent or arbitrary organization IDs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(api): audit preflight failure no longer blocks requests + filter expired API keys 1. Wrap audit preflight in .catch() so a failure in pre-flight data collection (e.g., bad controlIds) never blocks the actual API request. The request proceeds with empty audit context instead. 2. Filter expired API keys at the DB level to reduce payload size during validation. The full-table scan design (per-key salts) is a known limitation that requires a schema migration to fully resolve. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(api): add keyPrefix for indexed API key lookup Store first 8 chars of the plaintext key as `keyPrefix` for O(1) indexed lookup instead of loading all active keys into memory. Backwards compatible: legacy keys without a prefix fall back to scanning only keys where keyPrefix IS NULL, and the prefix is backfilled on first successful validation so future lookups are fast. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(api): handle errors in @res() streaming endpoint + set isServiceToken explicitly 1. Wrap completions endpoint in try/catch since @res() bypasses NestJS exception filters. Errors now return proper JSON responses instead of hanging. 2. Explicitly set isServiceToken=false in API key and session auth handlers to prevent accidental fallthrough in PermissionGuard. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(api): validate domain format in trust portal to prevent path injection Add domain format validation in addCustomDomain and checkDnsRecords to ensure user-provided domain values can't inject path segments into Vercel API or DNS lookup URLs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(api): merge duplicate permissions, skip chat audit, add maxSteps, fix control mapping descriptions 1. PermissionGuard: merge actions for duplicate resources instead of overwriting, preventing silently dropped permission checks. 2. Assistant chat completions: add @SkipAuditLog() to prevent noisy "Created app" audit entries on every chat message. 3. Assistant chat: add maxSteps=5 to streamText so the model can synthesize tool results into natural language responses. 4. Audit log resolvers: extract resource name from URL path instead of hardcoding "policy". fetchControlIds now dynamically resolves the correct Prisma model. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: rBAC support and major security improvements Role-Based Access Control (RBAC) system with custom roles, permission guards, and audit logging across the platform. Key changes: - Hybrid auth guard for session, API key, service token - Permission guard with better-auth and custom roles - Granular resource:action permissions - Audit log interceptor with mutation tracking - API key prefix indexing for O(1) lookups - Service token org existence validation - Domain validation to prevent SSRF - Streaming endpoint error handling - AI assistant chat with permission-gated tools BREAKING CHANGE: all API endpoints now require RBAC permissions via @RequirePermission decorators. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryHigh Risk Overview Audit logging is centralized. A new New assistant streaming completions and attachment hardening. Build/config updates. Adds repo/AI guidance docs, updates ignores, adjusts API Docker/buildspec to include Written by Cursor Bugbot for commit c8349f7. This will update automatically on new commits. Configure here. |
| axios | ||
| .get(`https://networkcalc.com/api/dns/lookup/${domain}`) |
Check failure
Code scanning / CodeQL
Server-side request forgery Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 15 hours ago
General approach: ensure that user input cannot alter the structure of the URL being requested, either by (a) constraining it to a strict format and then normalizing/encoding before interpolation, or (b) mapping it to a server-controlled value (allow-list). The hostname must remain fixed and user input must not be able to introduce additional path segments, query parameters, or protocol/host changes.
Best fix here without changing functionality: keep validateDomain as the main guard, but add an explicit normalization step that (1) lowercases and trims the domain, and (2) uses encodeURIComponent when inserting it in the path. This guarantees that even if the validation pattern is relaxed in the future, the user value cannot break out of its intended path segment. We will:
- Add a small helper method
sanitizeDomainForDnsLookup(domain: string): stringtoTrustPortalServicethat:- calls
this.validateDomain(domain); - normalizes the string (e.g.,
trim()andtoLowerCase()).
- calls
- Use that sanitized value (and derive
rootDomainfrom it) for all threeaxios.getcalls, and wrap the interpolated segments withencodeURIComponent(...)so only a single, encoded path segment is ever used.
Concretely in apps/api/src/trust-portal/trust-portal.service.ts:
- Insert a new private method (within the class, near
validateDomain) implementingsanitizeDomainForDnsLookup. - Modify
checkDnsRecordsto:- call
const normalizedDomain = this.sanitizeDomainForDnsLookup(domain); - compute
const rootDomain = normalizedDomain.split('.').slice(-2).join('.'); - use
encodeURIComponent(normalizedDomain)andencodeURIComponent(rootDomain)in the threeaxios.getURLs instead of interpolating rawdomain/rootDomain.
- call
No imports are needed; encodeURIComponent is globally available in Node/TypeScript.
| @@ -892,6 +892,19 @@ | ||
| } | ||
|
|
||
| /** | ||
| * Normalize and validate a domain before using it in DNS lookup URLs. | ||
| * This ensures user input cannot alter the URL structure. | ||
| */ | ||
| private sanitizeDomainForDnsLookup(domain: string): string { | ||
| this.validateDomain(domain); | ||
| const normalized = domain.trim().toLowerCase(); | ||
| if (!normalized) { | ||
| throw new BadRequestException('Invalid domain format'); | ||
| } | ||
| return normalized; | ||
| } | ||
|
|
||
| /** | ||
| * DNS CNAME patterns for Vercel verification. | ||
| */ | ||
| private static readonly VERCEL_DNS_CNAME_PATTERN = | ||
| @@ -900,22 +913,30 @@ | ||
| /vercel-dns[^.]*\.com\.?$/i; | ||
|
|
||
| async checkDnsRecords(organizationId: string, domain: string) { | ||
| this.validateDomain(domain); | ||
| const normalizedDomain = this.sanitizeDomainForDnsLookup(domain); | ||
|
|
||
| const rootDomain = domain.split('.').slice(-2).join('.'); | ||
| const rootDomain = normalizedDomain.split('.').slice(-2).join('.'); | ||
|
|
||
| const [cnameResp, txtResp, vercelTxtResp] = await Promise.all([ | ||
| axios | ||
| .get(`https://networkcalc.com/api/dns/lookup/${domain}`) | ||
| .get( | ||
| `https://networkcalc.com/api/dns/lookup/${encodeURIComponent( | ||
| normalizedDomain, | ||
| )}`, | ||
| ) | ||
| .catch(() => null), | ||
| axios | ||
| .get( | ||
| `https://networkcalc.com/api/dns/lookup/${rootDomain}?type=TXT`, | ||
| `https://networkcalc.com/api/dns/lookup/${encodeURIComponent( | ||
| rootDomain, | ||
| )}?type=TXT`, | ||
| ) | ||
| .catch(() => null), | ||
| axios | ||
| .get( | ||
| `https://networkcalc.com/api/dns/lookup/_vercel.${rootDomain}?type=TXT`, | ||
| `https://networkcalc.com/api/dns/lookup/_vercel.${encodeURIComponent( | ||
| rootDomain, | ||
| )}?type=TXT`, | ||
| ) | ||
| .catch(() => null), | ||
| ]); |
| axios | ||
| .get( | ||
| `https://networkcalc.com/api/dns/lookup/${rootDomain}?type=TXT`, | ||
| ) |
Check failure
Code scanning / CodeQL
Server-side request forgery Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 15 hours ago
General approach: continue to disallow control of the hostname, and further constrain and normalize the path component derived from user input before constructing the URL. Ensure only well-formed domain names that match a strict pattern and length limits are accepted, and avoid relying on partially processed strings (like rootDomain) without re-validating.
Best fix without changing behavior: keep the existing VALID_DOMAIN_PATTERN but add extra safety checks:
- Enforce a reasonable max length for any domain string we accept.
- Normalize
domainandrootDomain(e.g., trim whitespace, lowercase). - Re-validate
rootDomainusing the same domain validation before using it in URLs. - Optionally, slightly tighten the regex to ensure no leading/trailing dots sneak in and that only ASCII characters are allowed (it already does this).
Concretely in apps/api/src/trust-portal/trust-portal.service.ts:
- Extend
validateDomainto:trim()and lowercase the input before validation.- Enforce a maximum length (e.g., 253 characters, the DNS limit).
- In
checkDnsRecords:- Normalize
domainviaconst normalizedDomain = domain.trim().toLowerCase();and callvalidateDomain(normalizedDomain)instead ofdomain. - Derive
rootDomainfromnormalizedDomain. - Call
this.validateDomain(rootDomain)before using it in the axios URLs.
- Normalize
This keeps functionality the same for valid domains but makes the taint analysis see that only validated, normalized values are used in URLs, and it adds a further safety margin against malformed input.
No new imports or external libraries are required.
| @@ -885,10 +885,16 @@ | ||
| /** Validate domain to prevent path injection in API URLs */ | ||
| private static readonly VALID_DOMAIN_PATTERN = /^(?:[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?\.)+[a-zA-Z]{2,}$/; | ||
|
|
||
| private validateDomain(domain: string): void { | ||
| if (!TrustPortalService.VALID_DOMAIN_PATTERN.test(domain)) { | ||
| private validateDomain(domain: string): string { | ||
| const normalized = domain.trim().toLowerCase(); | ||
| // RFC 1035/1123: maximum length of a full domain name is 253 characters | ||
| if (normalized.length === 0 || normalized.length > 253) { | ||
| throw new BadRequestException('Invalid domain format'); | ||
| } | ||
| if (!TrustPortalService.VALID_DOMAIN_PATTERN.test(normalized)) { | ||
| throw new BadRequestException('Invalid domain format'); | ||
| } | ||
| return normalized; | ||
| } | ||
|
|
||
| /** | ||
| @@ -900,22 +904,23 @@ | ||
| /vercel-dns[^.]*\.com\.?$/i; | ||
|
|
||
| async checkDnsRecords(organizationId: string, domain: string) { | ||
| this.validateDomain(domain); | ||
| const normalizedDomain = this.validateDomain(domain); | ||
|
|
||
| const rootDomain = domain.split('.').slice(-2).join('.'); | ||
| const rootDomain = normalizedDomain.split('.').slice(-2).join('.'); | ||
| const normalizedRootDomain = this.validateDomain(rootDomain); | ||
|
|
||
| const [cnameResp, txtResp, vercelTxtResp] = await Promise.all([ | ||
| axios | ||
| .get(`https://networkcalc.com/api/dns/lookup/${domain}`) | ||
| .get(`https://networkcalc.com/api/dns/lookup/${normalizedDomain}`) | ||
| .catch(() => null), | ||
| axios | ||
| .get( | ||
| `https://networkcalc.com/api/dns/lookup/${rootDomain}?type=TXT`, | ||
| `https://networkcalc.com/api/dns/lookup/${normalizedRootDomain}?type=TXT`, | ||
| ) | ||
| .catch(() => null), | ||
| axios | ||
| .get( | ||
| `https://networkcalc.com/api/dns/lookup/_vercel.${rootDomain}?type=TXT`, | ||
| `https://networkcalc.com/api/dns/lookup/_vercel.${normalizedRootDomain}?type=TXT`, | ||
| ) | ||
| .catch(() => null), | ||
| ]); |
| axios | ||
| .get( | ||
| `https://networkcalc.com/api/dns/lookup/_vercel.${rootDomain}?type=TXT`, | ||
| ) |
Check failure
Code scanning / CodeQL
Server-side request forgery Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 15 hours ago
General fix approach
Mitigate SSRF by strictly validating and normalizing user-provided domain values before using them in any outgoing URL, and ensure they cannot affect host, scheme, or path traversal to unintended APIs. In practice here, that means:
- Enforcing a strict allow‑pattern for domains (already present via
VALID_DOMAIN_PATTERN). - Normalizing to lower case and stripping trailing dots.
- Ensuring the computed
rootDomainis structurally valid and still matches the allowed pattern. - (Optionally) using URL parameterization instead of string interpolation, but host remains constant.
Best concrete fix here
We already call this.validateDomain(domain); which uses VALID_DOMAIN_PATTERN. To make the flow clearly safe:
- Normalize
domainafter validation:- Convert to lower case.
- Strip any trailing dot.
- Derive
rootDomainfrom the normalized domain. - Explicitly re‑validate
rootDomainto ensure it is also a valid domain. - Use the normalized values in the axios calls.
This preserves existing behavior (domains that passed before will still pass, except possibly some pathological edge cases the regex accidentally allowed) and makes it clear that only well‑formed, public‑style hostnames are ever used in the external DNS lookup URLs.
Where and what to change
In apps/api/src/trust-portal/trust-portal.service.ts:
- Inside
checkDnsRecords:- Replace the current
const rootDomain = domain.split('.').slice(-2).join('.');with:- Normalization of
domaininto a new localnormalizedDomain. - Computation of
rootDomainfromnormalizedDomain. - A guard that throws
BadRequestExceptionifrootDomaindoes not matchVALID_DOMAIN_PATTERN.
- Normalization of
- Update the three
axios.getcalls to usenormalizedDomainandrootDomainvariables as appropriate (they already do logically, but we ensure they reference the normalized versions).
- Replace the current
No new helper functions or imports are needed; we only use existing BadRequestException and the static regex VALID_DOMAIN_PATTERN on TrustPortalService.
| @@ -902,11 +902,19 @@ | ||
| async checkDnsRecords(organizationId: string, domain: string) { | ||
| this.validateDomain(domain); | ||
|
|
||
| const rootDomain = domain.split('.').slice(-2).join('.'); | ||
| // Normalize the domain to a canonical, safe representation | ||
| const normalizedDomain = domain.toLowerCase().replace(/\.$/, ''); | ||
|
|
||
| const rootDomain = normalizedDomain.split('.').slice(-2).join('.'); | ||
|
|
||
| // Ensure the derived root domain is also a valid domain | ||
| if (!TrustPortalService.VALID_DOMAIN_PATTERN.test(rootDomain)) { | ||
| throw new BadRequestException('Invalid domain format'); | ||
| } | ||
|
|
||
| const [cnameResp, txtResp, vercelTxtResp] = await Promise.all([ | ||
| axios | ||
| .get(`https://networkcalc.com/api/dns/lookup/${domain}`) | ||
| .get(`https://networkcalc.com/api/dns/lookup/${normalizedDomain}`) | ||
| .catch(() => null), | ||
| axios | ||
| .get( |
| // No permissions required - allow access | ||
| if (!requiredPermissions || requiredPermissions.length === 0) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
PermissionGuard doesn't check @Public() decorator, blocks public routes
Medium Severity
PermissionGuard doesn't check for @Public() metadata. When a controller has @UseGuards(HybridAuthGuard, PermissionGuard) and @RequirePermission(...) at the class level, any method decorated with @Public() will pass HybridAuthGuard (which checks IS_PUBLIC_KEY) but then PermissionGuard will attempt to enforce the class-level permission — failing because no auth context was populated. This makes @Public() incompatible with class-level permission declarations, which could break webhook endpoints added to controllers with class-level RBAC.
Additional Locations (1)
…tegration (#2226) - Add @comp/auth package to Dockerfile and buildspec for proper inclusion in the Docker build process. - Remove workspace dependency for @comp/auth from package.json during build. - Ensure all necessary files are copied to the Docker image for the new authentication package.
| `; | ||
|
|
||
| const result = streamText({ | ||
| model: openai('gpt-5'), |
There was a problem hiding this comment.
AI model identifier may not exist yet
Medium Severity
The streamText call uses openai('gpt-5') as the model identifier. If gpt-5 is not a valid model name in the OpenAI API at deployment time, every assistant chat completion request will fail with an API error. There's no fallback model configured, so the entire assistant chat feature would be broken. Previous code didn't have a completions endpoint, so there's no prior model name to reference for correctness.
| } catch { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Audit log fetchCurrentValues lacks organizationId tenant scoping
Low Severity
fetchCurrentValues and fetchControlIds query database models by id alone without scoping by organizationId. While the entity ID originates from an already-authenticated request, multi-tenant isolation best practice requires always scoping queries. If an entity ID from another tenant were somehow injected, previous values from a different organization could leak into audit log entries.
Additional Locations (1)
|
|
||
| for (const [key, newValue] of Object.entries(body)) { | ||
| const previousRaw = previousValues?.[key]; | ||
|
|
There was a problem hiding this comment.
String coercion causes incorrect null equality in change tracking
Low Severity
buildChanges compares previous and new values using String(previousRaw) === String(newValue). Since String(null) is "null" and String(undefined) is "undefined", this incorrectly treats the literal string "null" as equal to a null database value, and similarly for "undefined". Genuine changes involving these values would be silently dropped from audit logs.
- Add @aws-sdk/client-ec2 and @thallesp/nestjs-better-auth to package.json and bun.lock. - Update assistant-chat.controller.ts to use stepCountIs for maxSteps in chat completions. - Refactor audit-log.resolvers.ts to fix type casting for database model access.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| } catch (verifyError: any) { | ||
| // If we get a key mismatch error, retry with a fresh JWKS fetch | ||
| if ( | ||
| verifyError.code === 'ERR_JWKS_NO_MATCHING_KEY' || |
There was a problem hiding this comment.
Service token auth skips member lookup, allowing access without membership
Medium Severity
handleServiceTokenAuth validates that the x-organization-id header references an existing organization but does not verify membership or any relationship between the service token and that specific organization. Any valid service token (e.g., the portal's token) can pass any x-organization-id header and access data for any organization within its scoped permissions. While service tokens are internal, a compromised or misconfigured service token gains cross-tenant access.
Additional Locations (1)
| }); | ||
|
|
||
| return result.success === true; | ||
| } |
There was a problem hiding this comment.
Double session resolution on every permission-gated request
Medium Severity
HybridAuthGuard calls auth.api.getSession to resolve the session, then PermissionGuard.checkPermission calls auth.api.hasPermission which resolves the same session again from the same headers. Every permission-gated endpoint incurs two full session resolution round-trips to better-auth (including DB lookups), doubling auth overhead on every request.


This is an automated pull request to release the candidate branch into production, which will trigger a deployment.
It was created by the [Production PR] action.