Skip to content

[comp] Production Deploy#2225

Open
github-actions[bot] wants to merge 4 commits intoreleasefrom
main
Open

[comp] Production Deploy#2225
github-actions[bot] wants to merge 4 commits intoreleasefrom
main

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Mar 5, 2026

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.

github-actions bot and others added 2 commits March 5, 2026 20:41
* 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>
@vercel
Copy link

vercel bot commented Mar 5, 2026

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

Project Deployment Actions Updated (UTC)
app (staging) Ready Ready Preview, Comment Mar 5, 2026 9:56pm
portal (staging) Ready Ready Preview, Comment Mar 5, 2026 9:56pm

Request Review

@cursor
Copy link

cursor bot commented Mar 5, 2026

PR Summary

High Risk
High risk because it replaces core API authentication/authorization flows (JWT→better-auth session + new PermissionGuard/@RequirePermission) and introduces a global audit-log interceptor plus new AI streaming endpoint, all of which can affect access control and request handling across the API.

Overview
Auth/RBAC is reworked across the API. HybridAuthGuard now authenticates via API keys (with optional scoped permissions), service-to-service tokens, or better-auth sessions (cookies/bearer), and controllers start using PermissionGuard + @RequirePermission for explicit endpoint protection (including new AuthController endpoints).

Audit logging is centralized. A new AuditModule registers a global AuditLogInterceptor that logs mutation (and opt-in read) activity with redaction and richer descriptions/diffs, plus a new GET /v1/audit-logs endpoint and @SkipAuditLog/@AuditRead decorators.

New assistant streaming completions and attachment hardening. POST /v1/assistant-chat/completions streams OpenAI responses with permission-gated tools (policies/risks), and attachments download is now guarded with evidence:read alongside new attachment helper methods.

Build/config updates. Adds repo/AI guidance docs, updates ignores, adjusts API Docker/buildspec to include @comp/auth, and updates API dependencies/env example for service tokens and new libraries.

Written by Cursor Bugbot for commit c8349f7. This will update automatically on new commits. Configure here.

Comment on lines +908 to +909
axios
.get(`https://networkcalc.com/api/dns/lookup/${domain}`)

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The
URL
of this request depends on a
user-provided value
.

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:

  1. Add a small helper method sanitizeDomainForDnsLookup(domain: string): string to TrustPortalService that:
    • calls this.validateDomain(domain);
    • normalizes the string (e.g., trim() and toLowerCase()).
  2. Use that sanitized value (and derive rootDomain from it) for all three axios.get calls, and wrap the interpolated segments with encodeURIComponent(...) 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) implementing sanitizeDomainForDnsLookup.
  • Modify checkDnsRecords to:
    • call const normalizedDomain = this.sanitizeDomainForDnsLookup(domain);
    • compute const rootDomain = normalizedDomain.split('.').slice(-2).join('.');
    • use encodeURIComponent(normalizedDomain) and encodeURIComponent(rootDomain) in the three axios.get URLs instead of interpolating raw domain / rootDomain.

No imports are needed; encodeURIComponent is globally available in Node/TypeScript.


Suggested changeset 1
apps/api/src/trust-portal/trust-portal.service.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/apps/api/src/trust-portal/trust-portal.service.ts b/apps/api/src/trust-portal/trust-portal.service.ts
--- a/apps/api/src/trust-portal/trust-portal.service.ts
+++ b/apps/api/src/trust-portal/trust-portal.service.ts
@@ -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),
     ]);
EOF
@@ -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),
]);
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +911 to +914
axios
.get(
`https://networkcalc.com/api/dns/lookup/${rootDomain}?type=TXT`,
)

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The
URL
of this request depends on a
user-provided value
.

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:

  1. Enforce a reasonable max length for any domain string we accept.
  2. Normalize domain and rootDomain (e.g., trim whitespace, lowercase).
  3. Re-validate rootDomain using the same domain validation before using it in URLs.
  4. 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 validateDomain to:
    • trim() and lowercase the input before validation.
    • Enforce a maximum length (e.g., 253 characters, the DNS limit).
  • In checkDnsRecords:
    • Normalize domain via const normalizedDomain = domain.trim().toLowerCase(); and call validateDomain(normalizedDomain) instead of domain.
    • Derive rootDomain from normalizedDomain.
    • Call this.validateDomain(rootDomain) before using it in the axios URLs.

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.

Suggested changeset 1
apps/api/src/trust-portal/trust-portal.service.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/apps/api/src/trust-portal/trust-portal.service.ts b/apps/api/src/trust-portal/trust-portal.service.ts
--- a/apps/api/src/trust-portal/trust-portal.service.ts
+++ b/apps/api/src/trust-portal/trust-portal.service.ts
@@ -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),
     ]);
EOF
@@ -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),
]);
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +916 to +919
axios
.get(
`https://networkcalc.com/api/dns/lookup/_vercel.${rootDomain}?type=TXT`,
)

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The
URL
of this request depends on a
user-provided value
.

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 rootDomain is 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:

  1. Normalize domain after validation:
    • Convert to lower case.
    • Strip any trailing dot.
  2. Derive rootDomain from the normalized domain.
  3. Explicitly re‑validate rootDomain to ensure it is also a valid domain.
  4. 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 domain into a new local normalizedDomain.
      • Computation of rootDomain from normalizedDomain.
      • A guard that throws BadRequestException if rootDomain does not match VALID_DOMAIN_PATTERN.
    • Update the three axios.get calls to use normalizedDomain and rootDomain variables as appropriate (they already do logically, but we ensure they reference the normalized versions).

No new helper functions or imports are needed; we only use existing BadRequestException and the static regex VALID_DOMAIN_PATTERN on TrustPortalService.


Suggested changeset 1
apps/api/src/trust-portal/trust-portal.service.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/apps/api/src/trust-portal/trust-portal.service.ts b/apps/api/src/trust-portal/trust-portal.service.ts
--- a/apps/api/src/trust-portal/trust-portal.service.ts
+++ b/apps/api/src/trust-portal/trust-portal.service.ts
@@ -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(
EOF
@@ -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(
Copilot is powered by AI and may make mistakes. Always verify output.
// No permissions required - allow access
if (!requiredPermissions || requiredPermissions.length === 0) {
return true;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Fix in Cursor Fix in Web

…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.
@vercel vercel bot temporarily deployed to staging – portal March 5, 2026 21:47 Inactive
@vercel vercel bot temporarily deployed to staging – app March 5, 2026 21:47 Inactive
`;

const result = streamText({
model: openai('gpt-5'),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Fix in Cursor Fix in Web

} catch {
return null;
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Fix in Cursor Fix in Web


for (const [key, newValue] of Object.entries(body)) {
const previousRaw = previousValues?.[key];

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Fix in Cursor Fix in Web

- 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.
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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' ||
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Fix in Cursor Fix in Web

});

return result.success === true;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Additional Locations (1)

Fix in Cursor Fix in Web

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.

1 participant