Skip to content

Commit 294b5bc

Browse files
committed
fix(ssrf-safety): restore SSRF safety checks and enhance diagnostics for URL validation
1 parent 869e97a commit 294b5bc

4 files changed

Lines changed: 79 additions & 12 deletions

File tree

backend/src/entities/table-actions/table-actions-module/table-action-activation.service.ts

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,12 @@ import { TableActionMethodEnum } from '../../../enums/table-action-method-enum.j
1010
import { ConnectionNotFoundException } from '../../../exceptions/custom-exceptions/connection-not-found-exception.js';
1111
import { Messages } from '../../../exceptions/text/messages.js';
1212
import { isSaaS } from '../../../helpers/app/is-saas.js';
13+
import { Constants } from '../../../helpers/constants/constants.js';
1314
import { Encryptor } from '../../../helpers/encryption/encryptor.js';
1415
import { actionSlackPostMessage } from '../../../helpers/slack/action-slack-post-message.js';
16+
import { slackPostMessage } from '../../../helpers/slack/slack-post-message.js';
1517
import { isObjectPropertyExists } from '../../../helpers/validators/is-object-property-exists-validator.js';
16-
// TODO: temporarily disabled SSRF/URL safety check in activateTableAction. Restore import to re-enable.
17-
// import { getSsrfSafeRequestConfig } from '../../../helpers/validators/ssrf-safe-http.js';
18+
import { getSsrfSafeRequestConfig } from '../../../helpers/validators/ssrf-safe-http.js';
1819
import { ConnectionEntity } from '../../connection/connection.entity.js';
1920
import { EmailService } from '../../email/email/email.service.js';
2021
import { escapeHtml } from '../../email/utils/escape-html.util.js';
@@ -212,9 +213,7 @@ export class TableActionActivationService {
212213
let result: AxiosResponse<any, any> | undefined;
213214
try {
214215
result = await axios.post(tableAction.url, actionRequestBody, {
215-
// TODO: SSRF/URL safety check temporarily disabled. Restore the line below to re-enable.
216-
// ...getSsrfSafeRequestConfig(),
217-
timeout: 10_000,
216+
...getSsrfSafeRequestConfig(),
218217
headers: { 'Rocketadmin-Signature': autoadminSignatureHeader, 'Content-Type': 'application/json' },
219218
maxRedirects: 0,
220219
validateStatus: (status) => status <= 599,
@@ -225,6 +224,23 @@ export class TableActionActivationService {
225224
console.info('HTTP action result headers', result?.headers);
226225
}
227226
} catch (error) {
227+
// TODO: temporary diagnostics for the SSRF safety check. A URL blocked by the guard surfaces
228+
// here as a request failure carrying the "SSRF guard" message. Report those to the errors
229+
// channel so we can confirm the guard is not rejecting legitimate action URLs, then remove.
230+
const errorMessage = error instanceof Error ? error.message : String(error);
231+
if (errorMessage.includes('SSRF guard')) {
232+
const host = (() => {
233+
try {
234+
return new URL(tableAction.url).host;
235+
} catch {
236+
return tableAction.url;
237+
}
238+
})();
239+
slackPostMessage(
240+
`[ssrf-check] Table action URL validation failed for host "${host}": ${errorMessage}`,
241+
Constants.EXCEPTIONS_CHANNELS,
242+
).catch(() => undefined);
243+
}
228244
if (axios.isAxiosError(error)) {
229245
const errorMessage =
230246
result?.data?.error ||

backend/src/helpers/constants/constants.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,11 @@ export const Constants = {
3333
'192.168.0.0/16',
3434
'127.0.0.0/8',
3535
'169.254.0.0/16',
36+
'100.64.0.0/10',
37+
'192.0.0.0/24',
38+
'198.18.0.0/15',
3639
'::1/128',
40+
'::/128',
3741
'fd00::/8',
3842
'fe80::/10',
3943
],

backend/src/helpers/validators/ssrf-safe-http.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,25 @@ export const ssrfGuardLookup: net.LookupFunction = (hostname, options, callback)
1515
callback(err, '', 0);
1616
return;
1717
}
18-
const forbidden = addresses.find(({ address }) => isForbiddenAddress(address));
19-
if (forbidden) {
20-
callback(new Error(`SSRF guard: refusing to connect to ${forbidden.address} for host ${hostname}`), '', 0);
18+
// Keep only the public addresses. We deliberately do NOT reject the whole host just because one of
19+
// its records is private (split-horizon DNS, stray records); we simply never connect to a forbidden
20+
// one. The request is refused only when no usable address remains.
21+
const allowed = addresses.filter(({ address }) => !isForbiddenAddress(address));
22+
if (allowed.length === 0) {
23+
callback(
24+
new Error(`SSRF guard: refusing to connect to ${hostname}; all resolved addresses are forbidden`),
25+
'',
26+
0,
27+
);
2128
return;
2229
}
23-
const first = addresses[0];
24-
if (!first) {
25-
callback(new Error(`SSRF guard: no addresses resolved for host ${hostname}`), '', 0);
30+
// Honour the "all" form so Node keeps every allowed address and can fall back across them
31+
// (Happy Eyeballs / IPv6-first hosts on IPv4-only egress). Otherwise return the first allowed one.
32+
if (options.all) {
33+
callback(null, allowed);
2634
return;
2735
}
36+
const first = allowed[0];
2837
callback(null, first.address, first.family);
2938
});
3039
};

backend/test/ava-tests/unit-tests/ssrf-safe-http.test.ts

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,31 @@ function lookup(host: string): Promise<{ address: string; family: number }> {
3232
});
3333
}
3434

35+
function lookupAll(host: string): Promise<Array<{ address: string; family: number }>> {
36+
return new Promise((resolve, reject) => {
37+
ssrfGuardLookup(host, { all: true }, (err, addresses) => {
38+
if (err) {
39+
reject(err);
40+
} else {
41+
resolve(addresses as Array<{ address: string; family: number }>);
42+
}
43+
});
44+
});
45+
}
46+
3547
// --- the validating lookup blocks internal targets, allows public ones --------------------------
3648

37-
for (const host of ['127.0.0.1', '::1', '169.254.169.254', '10.0.0.5', '192.168.1.1', '::ffff:127.0.0.1']) {
49+
for (const host of [
50+
'127.0.0.1',
51+
'::1',
52+
'169.254.169.254',
53+
'10.0.0.5',
54+
'192.168.1.1',
55+
'::ffff:127.0.0.1',
56+
'100.64.0.1', // carrier-grade NAT
57+
'198.18.0.1', // benchmarking
58+
'192.0.0.1', // IETF protocol assignments
59+
]) {
3860
test(`ssrfGuardLookup rejects forbidden address ${host}`, async (t) => {
3961
await t.throwsAsync(() => lookup(host), { message: /SSRF guard/ });
4062
});
@@ -50,6 +72,22 @@ test('ssrfGuardLookup resolves a public IPv6 address', async (t) => {
5072
t.is(address, '2606:4700:4700::1111');
5173
});
5274

75+
// --- the "all" form keeps every allowed address so Node can fall back across them -----------------
76+
77+
test('ssrfGuardLookup returns the allowed address as an array when all=true', async (t) => {
78+
const addresses = await lookupAll('8.8.8.8');
79+
t.true(Array.isArray(addresses));
80+
t.deepEqual(
81+
addresses.map((a) => a.address),
82+
['8.8.8.8'],
83+
);
84+
});
85+
86+
test('ssrfGuardLookup (all=true) rejects when every resolved address is forbidden', async (t) => {
87+
// localhost resolves to 127.0.0.1 and ::1 — a multi-record host where nothing is usable.
88+
await t.throwsAsync(() => lookupAll('localhost'), { message: /all resolved addresses are forbidden/ });
89+
});
90+
5391
// --- gating: pinning config is applied only in SaaS, non-test ----------------------------------
5492

5593
test.serial('getSsrfSafeRequestConfig returns pinning agents + guards in SaaS mode', (t) => {

0 commit comments

Comments
 (0)