Skip to content

Commit 9ccfa42

Browse files
MarceloRGoncMarceloRGonc
authored
Allow internal webhook call on the HTTP action (#2212)
Fixes OPS-4101
1 parent dc638c3 commit 9ccfa42

8 files changed

Lines changed: 210 additions & 111 deletions

File tree

‎packages/blocks/http/jest.config.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
export default {
2+
displayName: 'blocks-http',
3+
preset: '../../../jest.preset.js',
4+
setupFiles: ['../../../jest.env.js'],
5+
testEnvironment: 'node',
6+
transform: {
7+
'^.+\\.[tj]s$': ['ts-jest', { tsconfig: '<rootDir>/tsconfig.spec.json' }],
8+
},
9+
moduleFileExtensions: ['ts', 'js', 'html'],
10+
coverageDirectory: '../../../coverage/packages/blocks/http',
11+
};

‎packages/blocks/http/project.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@
1717
"updateBuildableProjectDepsInPackageJson": true
1818
}
1919
},
20+
"test": {
21+
"executor": "@nx/jest:jest",
22+
"outputs": ["{workspaceRoot}/coverage/{projectRoot}"],
23+
"options": {
24+
"jestConfig": "packages/blocks/http/jest.config.ts"
25+
}
26+
},
2027
"lint": {
2128
"executor": "@nx/eslint:lint",
2229
"outputs": ["{options.outputFile}"]

‎packages/blocks/http/src/lib/actions/send-http-request-action.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,14 @@ import {
1010
DynamicPropsValue,
1111
Property,
1212
} from '@openops/blocks-framework';
13-
import { validateHostAllowingPublicWebhookUrl } from '@openops/server-shared';
13+
import { validateHost } from '@openops/server-shared';
1414
import { assertNotNullOrUndefined } from '@openops/shared';
1515
import axios from 'axios';
1616
import FormData from 'form-data';
1717
import { HttpsProxyAgent } from 'https-proxy-agent';
1818
import { httpAuth } from '../common/auth';
1919
import { httpMethodDropdown } from '../common/props';
20+
import { validateAndRewritePublicWebhookUrl } from '../common/webhook-url-validator';
2021

2122
const toLowerCaseKeys = (obj: HttpHeaders) =>
2223
Object.fromEntries(Object.entries(obj).map(([k, v]) => [k.toLowerCase(), v]));
@@ -170,10 +171,8 @@ export const httpSendRequestAction = createAction({
170171
assertNotNullOrUndefined(method, 'Method');
171172
assertNotNullOrUndefined(url, 'URL');
172173

173-
await validateHostAllowingPublicWebhookUrl(url);
174-
await validateHostAllowingPublicWebhookUrl(
175-
context.propsValue.proxy_settings?.proxy_host,
176-
);
174+
const newUrl = await validateAndRewritePublicWebhookUrl(url);
175+
await validateHost(context.propsValue.proxy_settings?.proxy_host);
177176

178177
const headersArray =
179178
(context.auth?.headers as
@@ -193,7 +192,7 @@ export const httpSendRequestAction = createAction({
193192

194193
const request: HttpRequest = {
195194
method,
196-
url,
195+
url: newUrl,
197196
headers: mergedHeaders,
198197
queryParams: (queryParams ?? {}) as QueryParams,
199198
timeout: timeout ? timeout * 1000 : 0,

‎packages/blocks/http/src/lib/common/webhook-url-validator.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import { networkUtls, validateHost } from '@openops/server-shared';
2+
3+
export async function validateAndRewritePublicWebhookUrl(
4+
userUrl: string,
5+
): Promise<string> {
6+
if (!userUrl) {
7+
return userUrl;
8+
}
9+
10+
try {
11+
await validateHost(userUrl);
12+
return userUrl;
13+
} catch (error) {
14+
const publicUrl = await networkUtls.getPublicUrl();
15+
const internalApiUrl = networkUtls.getInternalApiUrl();
16+
17+
const publicUrlObj = new URL(publicUrl);
18+
const internalUrlObj = new URL(internalApiUrl);
19+
const userUrlObj = new URL(userUrl);
20+
21+
if (userUrlObj.origin !== publicUrlObj.origin) {
22+
throw error;
23+
}
24+
25+
const internalBasePath = internalUrlObj.pathname.replace(/\/$/, '');
26+
let relativePath = userUrlObj.pathname;
27+
28+
if (
29+
internalBasePath &&
30+
internalBasePath !== '/' &&
31+
relativePath.startsWith(internalBasePath)
32+
) {
33+
relativePath = relativePath.slice(internalBasePath.length);
34+
}
35+
36+
if (!relativePath.startsWith('/')) {
37+
relativePath = `/${relativePath}`;
38+
}
39+
40+
if (!/^\/v1\/webhooks\/[0-9A-Za-z]{21}\/sync$/.test(relativePath)) {
41+
throw error;
42+
}
43+
44+
const rewrittenPath = `${internalBasePath}${relativePath}`.replace(
45+
/\/{2,}/g,
46+
'/',
47+
);
48+
49+
return `${internalUrlObj.origin}${rewrittenPath}`;
50+
}
51+
}

‎packages/blocks/http/test/webhook-url-validator.test.ts‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
import { networkUtls, validateHost } from '@openops/server-shared';
2+
import { validateAndRewritePublicWebhookUrl } from '../src/lib/common/webhook-url-validator';
3+
4+
jest.mock('@openops/server-shared', () => ({
5+
validateHost: jest.fn(),
6+
networkUtls: {
7+
getPublicUrl: jest.fn(),
8+
getInternalApiUrl: jest.fn(),
9+
},
10+
}));
11+
12+
describe('webhook-url-validator', () => {
13+
beforeEach(() => {
14+
jest.clearAllMocks();
15+
});
16+
17+
it('should return the original URL if it is empty', async () => {
18+
const result = await validateAndRewritePublicWebhookUrl('');
19+
expect(result).toBe('');
20+
});
21+
22+
it('should return the original URL if validateHost passes', async () => {
23+
(validateHost as jest.Mock).mockResolvedValue(undefined);
24+
const userUrl = 'https://example.com/webhook';
25+
const result = await validateAndRewritePublicWebhookUrl(userUrl);
26+
expect(result).toBe(userUrl);
27+
expect(validateHost).toHaveBeenCalledWith(userUrl);
28+
});
29+
30+
it('should rewrite the URL if it matches the public URL origin and valid webhook path', async () => {
31+
const error = new Error('Host must not be an internal address');
32+
(validateHost as jest.Mock).mockRejectedValue(error);
33+
(networkUtls.getPublicUrl as jest.Mock).mockResolvedValue(
34+
'https://public.openops.com',
35+
);
36+
(networkUtls.getInternalApiUrl as jest.Mock).mockReturnValue(
37+
'http://internal-api:3000',
38+
);
39+
40+
const userUrl =
41+
'https://public.openops.com/v1/webhooks/123456789012345678901/sync';
42+
const result = await validateAndRewritePublicWebhookUrl(userUrl);
43+
44+
expect(result).toBe(
45+
'http://internal-api:3000/v1/webhooks/123456789012345678901/sync',
46+
);
47+
});
48+
49+
it('should rewrite the URL when public URL has a base path', async () => {
50+
const error = new Error('Host must not be an internal address');
51+
(validateHost as jest.Mock).mockRejectedValue(error);
52+
(networkUtls.getPublicUrl as jest.Mock).mockResolvedValue(
53+
'https://openops.com/',
54+
);
55+
(networkUtls.getInternalApiUrl as jest.Mock).mockReturnValue(
56+
'http://internal-api:3000/api',
57+
);
58+
59+
const userUrl =
60+
'https://openops.com/api/v1/webhooks/123456789012345678901/sync';
61+
const result = await validateAndRewritePublicWebhookUrl(userUrl);
62+
63+
expect(result).toBe(
64+
'http://internal-api:3000/api/v1/webhooks/123456789012345678901/sync',
65+
);
66+
});
67+
68+
it('should throw the original error if origin does not match public URL origin', async () => {
69+
const error = new Error('Host must not be an internal address');
70+
(validateHost as jest.Mock).mockRejectedValue(error);
71+
(networkUtls.getPublicUrl as jest.Mock).mockResolvedValue(
72+
'https://public.openops.com',
73+
);
74+
(networkUtls.getInternalApiUrl as jest.Mock).mockReturnValue(
75+
'http://internal-api:3000',
76+
);
77+
78+
const userUrl =
79+
'https://other-domain.com/v1/webhooks/123456789012345678901/sync';
80+
81+
await expect(validateAndRewritePublicWebhookUrl(userUrl)).rejects.toThrow(
82+
error,
83+
);
84+
});
85+
86+
it('should throw the original error if the path does not match the webhook pattern', async () => {
87+
const error = new Error('Host must not be an internal address');
88+
(validateHost as jest.Mock).mockRejectedValue(error);
89+
(networkUtls.getPublicUrl as jest.Mock).mockResolvedValue(
90+
'https://public.openops.com',
91+
);
92+
(networkUtls.getInternalApiUrl as jest.Mock).mockReturnValue(
93+
'http://internal-api:3000',
94+
);
95+
96+
const userUrl = 'https://public.openops.com/v1/webhooks/invalid-id/sync';
97+
98+
await expect(validateAndRewritePublicWebhookUrl(userUrl)).rejects.toThrow(
99+
error,
100+
);
101+
});
102+
103+
it('should handle multiple slashes correctly during rewrite', async () => {
104+
const error = new Error('Host must not be an internal address');
105+
(validateHost as jest.Mock).mockRejectedValue(error);
106+
(networkUtls.getPublicUrl as jest.Mock).mockResolvedValue(
107+
'https://public.openops.com/',
108+
);
109+
(networkUtls.getInternalApiUrl as jest.Mock).mockReturnValue(
110+
'http://internal-api:3000/',
111+
);
112+
113+
const userUrl =
114+
'https://public.openops.com/v1/webhooks/123456789012345678901/sync';
115+
const result = await validateAndRewritePublicWebhookUrl(userUrl);
116+
117+
expect(result).toBe(
118+
'http://internal-api:3000/v1/webhooks/123456789012345678901/sync',
119+
);
120+
});
121+
});

‎packages/blocks/http/tsconfig.spec.json‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
{
2+
"extends": "./tsconfig.json",
3+
"compilerOptions": {
4+
"outDir": "../../../dist/out-tsc",
5+
"module": "commonjs",
6+
"types": ["jest", "node"]
7+
},
8+
"include": [
9+
"jest.config.ts",
10+
"src/**/*.test.ts",
11+
"src/**/*.spec.ts",
12+
"src/**/*.d.ts"
13+
]
14+
}

‎packages/server/shared/src/lib/host-validation/index.ts‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { promises as dns } from 'dns';
22
import ipRangeCheck from 'ip-range-check';
33
import { isIPv4, isIPv6 } from 'net';
4-
import { networkUtls } from '../network-utils';
54
import { SharedSystemProp, system } from '../system';
65

76
const internalV4Cidrs = [
@@ -64,27 +63,3 @@ export async function validateHost(host: string | undefined): Promise<void> {
6463
const isPrivate = await isInternalHost(host);
6564
if (isPrivate) throw new Error('Host must not be an internal address');
6665
}
67-
68-
export async function validateHostAllowingPublicWebhookUrl(
69-
url: string | undefined,
70-
): Promise<void> {
71-
if (!url) {
72-
return;
73-
}
74-
75-
try {
76-
await validateHost(url);
77-
} catch (error) {
78-
const publicUrl = await networkUtls.getPublicUrl();
79-
80-
const escapedBase = publicUrl.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
81-
82-
const regex = new RegExp(
83-
`^${escapedBase}v1/webhooks/[0-9a-zA-Z]{21}/sync$`,
84-
);
85-
86-
if (!regex.test(url)) {
87-
throw error;
88-
}
89-
}
90-
}

‎packages/server/shared/test/host-validation/index.test.ts‎

Lines changed: 1 addition & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
const resolve4 = jest.fn();
22
const resolve6 = jest.fn();
33
const getBoolean = jest.fn().mockReturnValue(true);
4-
const getPublicUrl = jest.fn();
54

65
jest.mock('dns', () => ({ promises: { resolve4, resolve6 } }));
76
jest.mock('../../src/lib/system', () => ({
@@ -15,14 +14,8 @@ jest.mock('../../src/lib/system', () => ({
1514
},
1615
AppSystemProp: { CLIENT_REAL_IP_HEADER: 'CLIENT_REAL_IP_HEADER' },
1716
}));
18-
jest.mock('../../src/lib/network-utils', () => ({
19-
networkUtls: { getPublicUrl },
20-
}));
2117

22-
import {
23-
validateHost,
24-
validateHostAllowingPublicWebhookUrl,
25-
} from '../../src/lib/host-validation';
18+
import { validateHost } from '../../src/lib/host-validation';
2619

2720
describe('Host Validation', () => {
2821
beforeEach(() => {
@@ -128,76 +121,4 @@ describe('Host Validation', () => {
128121
'Host must not be an internal address',
129122
);
130123
});
131-
132-
describe('validateHostAllowingPublicWebhookUrl', () => {
133-
test('should skip for empty url', async () => {
134-
const url = '';
135-
await expect(
136-
validateHostAllowingPublicWebhookUrl(url),
137-
).resolves.toBeUndefined();
138-
});
139-
140-
test('should allow public webhook url even if it resolves to private ip', async () => {
141-
const publicUrl = 'https://openops.example.com/';
142-
const webhookUrl =
143-
'https://openops.example.com/v1/webhooks/123456789012345678901/sync';
144-
getPublicUrl.mockResolvedValue(publicUrl);
145-
resolve4.mockResolvedValue(['127.0.0.1']);
146-
resolve6.mockResolvedValue([]);
147-
148-
await expect(
149-
validateHostAllowingPublicWebhookUrl(webhookUrl),
150-
).resolves.toBeUndefined();
151-
});
152-
153-
test('should throw for internal host that is not the public webhook url', async () => {
154-
const publicUrl = 'https://openops.example.com/';
155-
const internalUrl =
156-
'https://10.0.0.1/v1/webhooks/123456789012345678901/sync';
157-
getPublicUrl.mockResolvedValue(publicUrl);
158-
resolve4.mockResolvedValue(['10.0.0.1']);
159-
resolve6.mockResolvedValue([]);
160-
161-
await expect(
162-
validateHostAllowingPublicWebhookUrl(internalUrl),
163-
).rejects.toThrow('Host must not be an internal address');
164-
});
165-
166-
test('should throw for public webhook url with invalid id length', async () => {
167-
const publicUrl = 'https://openops.example.com/';
168-
const webhookUrl =
169-
'https://openops.example.com/v1/webhooks/too-short/sync';
170-
getPublicUrl.mockResolvedValue(publicUrl);
171-
resolve4.mockResolvedValue(['127.0.0.1']);
172-
resolve6.mockResolvedValue([]);
173-
174-
await expect(
175-
validateHostAllowingPublicWebhookUrl(webhookUrl),
176-
).rejects.toThrow('Host must not be an internal address');
177-
});
178-
179-
test('should allow public host', async () => {
180-
const publicUrl = 'https://openops.example.com/';
181-
const somePublicUrl = 'https://google.com';
182-
getPublicUrl.mockResolvedValue(publicUrl);
183-
resolve4.mockResolvedValue(['8.8.8.8']);
184-
resolve6.mockResolvedValue([]);
185-
186-
await expect(
187-
validateHostAllowingPublicWebhookUrl(somePublicUrl),
188-
).resolves.toBeUndefined();
189-
});
190-
191-
test('should throw error for DNS resolution failure on non-webhook URL', async () => {
192-
const publicUrl = 'https://openops.example.com/';
193-
const unknownUrl = 'https://unknown.example.com';
194-
getPublicUrl.mockResolvedValue(publicUrl);
195-
resolve4.mockRejectedValue(new Error('DNS resolution failed'));
196-
resolve6.mockRejectedValue(new Error('DNS resolution failed'));
197-
198-
await expect(
199-
validateHostAllowingPublicWebhookUrl(unknownUrl),
200-
).rejects.toThrow('Failed to resolve host');
201-
});
202-
});
203124
});

0 commit comments

Comments
 (0)