Skip to content

Commit 1afdc13

Browse files
committed
WIP
1 parent dc638c3 commit 1afdc13

8 files changed

Lines changed: 189 additions & 106 deletions

File tree

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,
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
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 internalUrl = new URL(networkUtls.getInternalApiUrl());
16+
const escapedBase = publicUrl.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
17+
18+
const regex = new RegExp(
19+
`^${escapedBase}v1/webhooks/[0-9a-zA-Z]{21}/sync$`,
20+
);
21+
22+
if (!regex.test(userUrl)) {
23+
throw error;
24+
}
25+
26+
const userUrlObj = new URL(userUrl);
27+
const publicUrlObj = new URL(publicUrl);
28+
29+
if (userUrlObj.origin !== publicUrlObj.origin) {
30+
return userUrl;
31+
}
32+
33+
userUrlObj.protocol = internalUrl.protocol;
34+
userUrlObj.hostname = internalUrl.hostname;
35+
userUrlObj.port = internalUrl.port;
36+
37+
const publicBasePath = publicUrlObj.pathname.replace(/\/$/, '');
38+
39+
if (
40+
publicBasePath &&
41+
publicBasePath !== '/' &&
42+
userUrlObj.pathname.startsWith(publicBasePath)
43+
) {
44+
const newPath = userUrlObj.pathname.slice(publicBasePath.length);
45+
userUrlObj.pathname = newPath.startsWith('/') ? newPath : `/${newPath}`;
46+
}
47+
48+
return userUrlObj.toString();
49+
}
50+
}
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
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 succeeds', 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+
describe('when validateHost fails', () => {
31+
const error = new Error('Host must not be an internal address');
32+
const publicUrl = 'https://app.openops.com/';
33+
const internalApiUrl = 'http://api-service:3000/';
34+
35+
beforeEach(() => {
36+
(validateHost as jest.Mock).mockRejectedValue(error);
37+
(networkUtls.getPublicUrl as jest.Mock).mockResolvedValue(publicUrl);
38+
(networkUtls.getInternalApiUrl as jest.Mock).mockReturnValue(
39+
internalApiUrl,
40+
);
41+
});
42+
43+
it('should re-throw the error if the URL does not match the webhook regex', async () => {
44+
const userUrl = 'https://internal.host/some-other-path';
45+
await expect(validateAndRewritePublicWebhookUrl(userUrl)).rejects.toThrow(
46+
error,
47+
);
48+
});
49+
50+
it('should re-throw the error if the URL matches regex but has different origin', async () => {
51+
// In reality, it will throw because it won't pass the regex (since regex includes publicUrl)
52+
// but let's test if we can somehow hit the "return userUrl" branch at line 30.
53+
// If publicUrl is 'https://app.openops.com/'
54+
// escapedBase is 'https:\/\/app\.openops\.com\/'
55+
// regex is '^https:\/\/app\.openops\.com\/v1/webhooks/[0-9a-zA-Z]{21}/sync$'
56+
// To pass regex, the URL MUST start with the publicUrl.
57+
// So let's see if origin comparison could still fail?
58+
// Maybe with case differences or port differences?
59+
// But URL origin and regex should match.
60+
// Let's test the main rewrite case.
61+
});
62+
63+
it('should rewrite the URL to internal API URL when it is a valid system webhook', async () => {
64+
const webhookId = 'abc123456789012345678';
65+
const userUrl = `${publicUrl}v1/webhooks/${webhookId}/sync`;
66+
67+
const result = await validateAndRewritePublicWebhookUrl(userUrl);
68+
69+
expect(result).toBe(
70+
`http://api-service:3000/v1/webhooks/${webhookId}/sync`,
71+
);
72+
});
73+
74+
it('should handle public URL with a base path correctly during rewrite', async () => {
75+
const basePublicUrl = 'https://openops.com/app/';
76+
const internalUrl = 'http://internal:3000/';
77+
(networkUtls.getPublicUrl as jest.Mock).mockResolvedValue(basePublicUrl);
78+
(networkUtls.getInternalApiUrl as jest.Mock).mockReturnValue(internalUrl);
79+
80+
const webhookId = 'abc123456789012345678';
81+
const userUrl = `${basePublicUrl}v1/webhooks/${webhookId}/sync`;
82+
83+
const result = await validateAndRewritePublicWebhookUrl(userUrl);
84+
85+
expect(result).toBe(`http://internal:3000/v1/webhooks/${webhookId}/sync`);
86+
});
87+
88+
it('should handle public URL with trailing slash in base path correctly', async () => {
89+
const basePublicUrl = 'https://openops.com/app/';
90+
const internalUrl = 'http://internal:3000/';
91+
(networkUtls.getPublicUrl as jest.Mock).mockResolvedValue(basePublicUrl);
92+
(networkUtls.getInternalApiUrl as jest.Mock).mockReturnValue(internalUrl);
93+
94+
const webhookId = 'abc123456789012345678';
95+
const userUrl = `https://openops.com/app/v1/webhooks/${webhookId}/sync`;
96+
97+
const result = await validateAndRewritePublicWebhookUrl(userUrl);
98+
expect(result).toBe(`http://internal:3000/v1/webhooks/${webhookId}/sync`);
99+
});
100+
});
101+
});
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 & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -64,27 +64,3 @@ export async function validateHost(host: string | undefined): Promise<void> {
6464
const isPrivate = await isInternalHost(host);
6565
if (isPrivate) throw new Error('Host must not be an internal address');
6666
}
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 & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,7 @@ jest.mock('../../src/lib/network-utils', () => ({
1919
networkUtls: { getPublicUrl },
2020
}));
2121

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

2724
describe('Host Validation', () => {
2825
beforeEach(() => {
@@ -128,76 +125,4 @@ describe('Host Validation', () => {
128125
'Host must not be an internal address',
129126
);
130127
});
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-
});
203128
});

0 commit comments

Comments
 (0)