Centralize outbound HTTP requests through SecureHttpClientService (#17779)
## Summary - Migrates all direct `axios` and `@nestjs/axios` `HttpService` usages across the server to go through `SecureHttpClientService`, which conditionally applies SSRF protection based on the `OUTBOUND_HTTP_SAFE_MODE_ENABLED` config flag - `SecureHttpClientService.getHttpClient()` now accepts optional `AxiosRequestConfig` (e.g., `baseURL`) so callers can configure their client while still getting protection - Adds `getInternalHttpClient()` for trusted same-server requests (e.g., REST-to-GraphQL proxy, code-interpreter downloading internal files) - Renames `getSecureAdapter` to `getSecureAxiosAdapter` for clarity - Captcha drivers now receive a pre-configured `AxiosInstance` from the module factory instead of creating their own ## Migrated services | Service | Previous | Risk level | |---------|----------|-----------| | `file-upload.service` | `HttpService` | High (user-provided image URLs) | | `code-interpreter-tool` | `HttpService` + direct adapter | High (user-provided file URLs) | | `search-help-center-tool` | `axios.post()` | Low (hardcoded endpoints) | | `http-tool` | Already migrated | High (user-provided URLs) | | `admin-panel.service` | `axios.get()` | Low (Docker Hub API) | | `sign-in-up.service` | `HttpService` | Medium (logo URL validation) | | `google-apis-scopes` | `HttpService` | Low (Google API) | | `geo-map.service` | `HttpService` | Low (Google Maps API) | | `telemetry.service` | `HttpService` | Low (telemetry endpoint) | | `rest-api.service` | `HttpService` | Internal (uses `getInternalHttpClient`) | | `create-company.service` | `axios.create()` | Low (Twenty companies API) | | `google-recaptcha.driver` | `axios.create()` | Low (Google reCAPTCHA) | | `turnstile.driver` | `axios.create()` | Low (Cloudflare Turnstile) | ## Test plan - [x] `npx nx typecheck twenty-server` passes - [x] `npx nx lint:diff-with-main twenty-server` passes - [x] Admin panel unit tests pass - [x] Secure adapter unit tests pass Made with [Cursor](https://cursor.com) --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
This commit is contained in:
+15
-10
@@ -1,13 +1,12 @@
|
||||
import { Test, type TestingModule } from '@nestjs/testing';
|
||||
import { getRepositoryToken } from '@nestjs/typeorm';
|
||||
|
||||
import axios from 'axios';
|
||||
|
||||
import { AdminPanelService } from 'src/engine/core-modules/admin-panel/admin-panel.service';
|
||||
import { AuditService } from 'src/engine/core-modules/audit/services/audit.service';
|
||||
import { LoginTokenService } from 'src/engine/core-modules/auth/token/services/login-token.service';
|
||||
import { WorkspaceDomainsService } from 'src/engine/core-modules/domain/workspace-domains/services/workspace-domains.service';
|
||||
import { FileService } from 'src/engine/core-modules/file/services/file.service';
|
||||
import { SecureHttpClientService } from 'src/engine/core-modules/tool/services/secure-http-client.service';
|
||||
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
|
||||
import { UserEntity } from 'src/engine/core-modules/user/user.entity';
|
||||
|
||||
@@ -15,6 +14,8 @@ const UserFindOneMock = jest.fn();
|
||||
const LoginTokenServiceGenerateLoginTokenMock = jest.fn();
|
||||
const TwentyConfigServiceGetAllMock = jest.fn();
|
||||
const TwentyConfigServiceGetVariableWithMetadataMock = jest.fn();
|
||||
const mockHttpClientGet = jest.fn();
|
||||
const mockGetHttpClient = jest.fn().mockReturnValue({ get: mockHttpClientGet });
|
||||
|
||||
jest.mock(
|
||||
'src/engine/core-modules/twenty-config/constants/config-variables-group-metadata',
|
||||
@@ -87,6 +88,12 @@ describe('AdminPanelService', () => {
|
||||
provide: FileService,
|
||||
useValue: {},
|
||||
},
|
||||
{
|
||||
provide: SecureHttpClientService,
|
||||
useValue: {
|
||||
getHttpClient: mockGetHttpClient,
|
||||
},
|
||||
},
|
||||
],
|
||||
}).compile();
|
||||
|
||||
@@ -256,18 +263,16 @@ describe('AdminPanelService', () => {
|
||||
|
||||
describe('getVersionInfo', () => {
|
||||
const mockEnvironmentGet = jest.fn();
|
||||
const mockAxiosGet = jest.fn();
|
||||
|
||||
beforeEach(() => {
|
||||
mockEnvironmentGet.mockReset();
|
||||
mockAxiosGet.mockReset();
|
||||
jest.spyOn(axios, 'get').mockImplementation(mockAxiosGet);
|
||||
mockHttpClientGet.mockReset();
|
||||
service['twentyConfigService'].get = mockEnvironmentGet;
|
||||
});
|
||||
|
||||
it('should return current and latest version when everything works', async () => {
|
||||
mockEnvironmentGet.mockReturnValue('1.0.0');
|
||||
mockAxiosGet.mockResolvedValue({
|
||||
mockHttpClientGet.mockResolvedValue({
|
||||
data: {
|
||||
results: [
|
||||
{ name: '2.0.0' },
|
||||
@@ -288,7 +293,7 @@ describe('AdminPanelService', () => {
|
||||
|
||||
it('should handle undefined APP_VERSION', async () => {
|
||||
mockEnvironmentGet.mockReturnValue(undefined);
|
||||
mockAxiosGet.mockResolvedValue({
|
||||
mockHttpClientGet.mockResolvedValue({
|
||||
data: {
|
||||
results: [{ name: '2.0.0' }, { name: 'latest' }],
|
||||
},
|
||||
@@ -304,7 +309,7 @@ describe('AdminPanelService', () => {
|
||||
|
||||
it('should handle Docker Hub API error', async () => {
|
||||
mockEnvironmentGet.mockReturnValue('1.0.0');
|
||||
mockAxiosGet.mockRejectedValue(new Error('API Error'));
|
||||
mockHttpClientGet.mockRejectedValue(new Error('API Error'));
|
||||
|
||||
const result = await service.getVersionInfo();
|
||||
|
||||
@@ -316,7 +321,7 @@ describe('AdminPanelService', () => {
|
||||
|
||||
it('should handle empty Docker Hub tags', async () => {
|
||||
mockEnvironmentGet.mockReturnValue('1.0.0');
|
||||
mockAxiosGet.mockResolvedValue({
|
||||
mockHttpClientGet.mockResolvedValue({
|
||||
data: {
|
||||
results: [],
|
||||
},
|
||||
@@ -332,7 +337,7 @@ describe('AdminPanelService', () => {
|
||||
|
||||
it('should handle invalid semver tags', async () => {
|
||||
mockEnvironmentGet.mockReturnValue('1.0.0');
|
||||
mockAxiosGet.mockResolvedValue({
|
||||
mockHttpClientGet.mockResolvedValue({
|
||||
data: {
|
||||
results: [
|
||||
{ name: '2.0.0' },
|
||||
|
||||
@@ -7,6 +7,7 @@ import { AdminPanelQueueService } from 'src/engine/core-modules/admin-panel/admi
|
||||
import { AdminPanelResolver } from 'src/engine/core-modules/admin-panel/admin-panel.resolver';
|
||||
import { AdminPanelService } from 'src/engine/core-modules/admin-panel/admin-panel.service';
|
||||
import { AuditModule } from 'src/engine/core-modules/audit/audit.module';
|
||||
import { SecureHttpClientService } from 'src/engine/core-modules/tool/services/secure-http-client.service';
|
||||
import { AuthModule } from 'src/engine/core-modules/auth/auth.module';
|
||||
import { WorkspaceDomainsModule } from 'src/engine/core-modules/domain/workspace-domains/workspace-domains.module';
|
||||
import { FeatureFlagModule } from 'src/engine/core-modules/feature-flag/feature-flag.module';
|
||||
@@ -38,6 +39,7 @@ import { PermissionsModule } from 'src/engine/metadata-modules/permissions/permi
|
||||
AdminPanelService,
|
||||
AdminPanelHealthService,
|
||||
AdminPanelQueueService,
|
||||
SecureHttpClientService,
|
||||
],
|
||||
exports: [AdminPanelService],
|
||||
})
|
||||
|
||||
@@ -2,7 +2,6 @@ import { Injectable } from '@nestjs/common';
|
||||
import { InjectRepository } from '@nestjs/typeorm';
|
||||
|
||||
import { msg } from '@lingui/core/macro';
|
||||
import axios from 'axios';
|
||||
import semver from 'semver';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { Repository } from 'typeorm';
|
||||
@@ -21,6 +20,7 @@ import { WorkspaceDomainsService } from 'src/engine/core-modules/domain/workspac
|
||||
import { FeatureFlagKey } from 'src/engine/core-modules/feature-flag/enums/feature-flag-key.enum';
|
||||
import { type FeatureFlagEntity } from 'src/engine/core-modules/feature-flag/feature-flag.entity';
|
||||
import { FileService } from 'src/engine/core-modules/file/services/file.service';
|
||||
import { SecureHttpClientService } from 'src/engine/core-modules/tool/services/secure-http-client.service';
|
||||
import { type ConfigVariables } from 'src/engine/core-modules/twenty-config/config-variables';
|
||||
import { CONFIG_VARIABLES_GROUP_METADATA } from 'src/engine/core-modules/twenty-config/constants/config-variables-group-metadata';
|
||||
import { type ConfigVariablesGroup } from 'src/engine/core-modules/twenty-config/enums/config-variables-group.enum';
|
||||
@@ -34,6 +34,7 @@ export class AdminPanelService {
|
||||
private readonly twentyConfigService: TwentyConfigService,
|
||||
private readonly workspaceDomainsService: WorkspaceDomainsService,
|
||||
private readonly fileService: FileService,
|
||||
private readonly secureHttpClientService: SecureHttpClientService,
|
||||
@InjectRepository(UserEntity)
|
||||
private readonly userRepository: Repository<UserEntity>,
|
||||
) {}
|
||||
@@ -185,7 +186,9 @@ export class AdminPanelService {
|
||||
const currentVersion = this.twentyConfigService.get('APP_VERSION');
|
||||
|
||||
try {
|
||||
const rawResponse = await axios.get<unknown>(
|
||||
const httpClient = this.secureHttpClientService.getHttpClient();
|
||||
|
||||
const rawResponse = await httpClient.get<unknown>(
|
||||
'https://hub.docker.com/v2/repositories/twentycrm/twenty/tags?page_size=100',
|
||||
);
|
||||
const response = z
|
||||
|
||||
Reference in New Issue
Block a user