diff --git a/packages/data-provider/specs/actions.spec.ts b/packages/data-provider/specs/actions.spec.ts index 59f068586d..cd03c1596d 100644 --- a/packages/data-provider/specs/actions.spec.ts +++ b/packages/data-provider/specs/actions.spec.ts @@ -1921,12 +1921,82 @@ describe('SSRF Protection', () => { expect(result.message).toContain('Failed to validate domain'); }); - it('validates with port numbers', () => { + it('preserves custom ports when the client domain omits a port', () => { const result = validateActionDomain('example.com', 'https://example.com:8443/api'); expect(result.isValid).toBe(true); expect(result.normalizedSpecDomain).toBe('https://example.com'); }); + it('rejects a same-host OpenAPI server on a different metadata port', () => { + const result = validateActionDomain( + 'https://example.com:8443', + 'https://example.com:9443/api', + ); + expect(result.isValid).toBe(false); + expect(result.message).toContain('Port mismatch'); + }); + + it('rejects a different port when metadata has trailing whitespace', () => { + const result = validateActionDomain( + 'https://example.com:8443 ', + 'https://example.com:9443/api', + ); + expect(result.isValid).toBe(false); + expect(result.message).toContain('Port mismatch'); + }); + + it.each([ + ['an ASCII newline', '\n/path'], + ['an ASCII tab', '\t/path'], + ['a backslash', '\\path'], + ])('rejects a different port when metadata contains %s', (_description, suffix) => { + const result = validateActionDomain( + `https://example.com:8443${suffix}`, + 'https://example.com:9443/api', + ); + expect(result.isValid).toBe(false); + expect(result.message).toContain('Port mismatch'); + }); + + it('rejects reserved port zero', () => { + const result = validateActionDomain('https://example.com:0', 'https://example.com:0/api'); + expect(result.isValid).toBe(false); + expect(result.message).toContain('Invalid port'); + }); + + it('rejects a same-IPv4 OpenAPI server on a different metadata port', () => { + const result = validateActionDomain('http://127.0.0.1:38079', 'http://127.0.0.1:38080/api'); + expect(result.isValid).toBe(false); + expect(result.message).toContain('Port mismatch'); + }); + + it('rejects a same-IPv6 OpenAPI server on a different metadata port', () => { + const result = validateActionDomain('http://[::1]:8080', 'http://[::1]:8081/api'); + expect(result.isValid).toBe(false); + expect(result.message).toContain('Port mismatch'); + }); + + it('validates matching custom ports for IPv6 domains', () => { + const result = validateActionDomain('http://[::1]:8080', 'http://[::1]:8080/api'); + expect(result.isValid).toBe(true); + }); + + it('matches an explicit HTTPS default port to an omitted server port', () => { + const result = validateActionDomain('https://example.com:443', 'https://example.com/api'); + expect(result.isValid).toBe(true); + }); + + it('matches an explicit HTTP default port to an omitted server port', () => { + const result = validateActionDomain('http://example.com:80', 'http://example.com/api'); + expect(result.isValid).toBe(true); + }); + + it('rejects an omitted server port when metadata specifies a custom port', () => { + const result = validateActionDomain('https://example.com:8443', 'https://example.com/api'); + expect(result.isValid).toBe(false); + expect(result.message).toContain('Port mismatch'); + }); + it('detects port-based SSRF attempt', () => { const result = validateActionDomain('example.com', 'http://example.com:6379/'); expect(result.isValid).toBe(false); @@ -2503,9 +2573,8 @@ describe('SSRF Protection', () => { it('handles client domain with port and no protocol', () => { const result = validateActionDomain('example.com:443', 'https://example.com:443/api'); - // Port is included in hostname comparison, causing mismatch - expect(result.isValid).toBe(false); - expect(result.normalizedClientDomain).toBe('https://example.com:443'); + expect(result.isValid).toBe(true); + expect(result.normalizedClientDomain).toBe('https://example.com'); expect(result.normalizedSpecDomain).toBe('https://example.com'); }); diff --git a/packages/data-provider/src/actions.ts b/packages/data-provider/src/actions.ts index 53c9e8ae1c..dd9a674444 100644 --- a/packages/data-provider/src/actions.ts +++ b/packages/data-provider/src/actions.ts @@ -3,10 +3,10 @@ import { URL } from 'url'; import _axios from 'axios'; import crypto from 'crypto'; import { load } from 'js-yaml'; +import type { OpenAPIV3 } from 'openapi-types'; import type { ActionMetadata, ActionMetadataRuntime } from './types/agents'; import type { FunctionTool, Schema, Reference } from './types/assistants'; import { AuthTypeEnum, AuthorizationTypeEnum } from './types/agents'; -import type { OpenAPIV3 } from 'openapi-types'; import { Tools } from './types/assistants'; export type ParametersSchema = { @@ -636,6 +636,45 @@ export type DomainValidationResult = { normalizedClientDomain?: string; }; +function getExplicitPort(value: string): string | null { + const normalizedValue = value.trim(); + const protocolSeparatorIndex = normalizedValue.indexOf('://'); + const hasProtocol = protocolSeparatorIndex !== -1; + const authorityAndPath = hasProtocol + ? normalizedValue.slice(protocolSeparatorIndex + 3) + : normalizedValue; + + let port: string; + try { + const parsedUrl = new URL(hasProtocol ? normalizedValue : `https://${normalizedValue}`); + port = parsedUrl.port; + + // WHATWG URL parsing removes explicit default ports, so parse the same authority with + // the alternate HTTP scheme to distinguish an explicit port from an omitted one. + if (!port && (parsedUrl.protocol === 'http:' || parsedUrl.protocol === 'https:')) { + const alternateProtocol = parsedUrl.protocol === 'http:' ? 'https:' : 'http:'; + port = new URL(`${alternateProtocol}//${authorityAndPath}`).port; + } + } catch { + return null; + } + + if (!port) { + return null; + } + + const parsedPort = Number(port); + if (!Number.isInteger(parsedPort) || parsedPort < 1 || parsedPort > 65535) { + throw new Error(`Invalid port in domain: ${value}`); + } + + return String(parsedPort); +} + +function getDefaultActionPort(protocol: string): string { + return protocol === 'https:' ? '443' : '80'; +} + /** * Validates client domain matches OpenAPI spec server URL domain (SSRF prevention). * @param clientProvidedDomain - Domain from client (with/without protocol) @@ -669,6 +708,7 @@ export function validateActionDomain( /** Extract hostname from client domain if it's a full URL */ let clientHostname = clientProvidedDomain; let clientHasProtocol = false; + const clientExplicitPort = getExplicitPort(clientProvidedDomain); // Check for any protocol in the client domain if (clientProvidedDomain.includes('://')) { @@ -689,6 +729,9 @@ export function validateActionDomain( // If parsing fails, treat as hostname clientHasProtocol = false; } + } else if (clientExplicitPort !== null) { + const clientUrl = new URL(`https://${clientProvidedDomain}`); + clientHostname = clientUrl.hostname; } /** Normalize IPv6 addresses by removing brackets for comparison */ @@ -718,20 +761,33 @@ export function validateActionDomain( } } - if ( + const domainMatches = normalizedSpecDomain === normalizedClientDomain || - (!clientHasProtocol && isIPAddress && normalizedClientHostname === normalizedSpecHostname) - ) { + (!clientHasProtocol && isIPAddress && normalizedClientHostname === normalizedSpecHostname); + + if (!domainMatches) { return { - isValid: true, + isValid: false, + message: `Domain mismatch: Client provided '${clientProvidedDomain}', but spec uses '${specHostname}'`, normalizedSpecDomain, normalizedClientDomain, }; } + if (clientExplicitPort !== null) { + const specEffectivePort = specUrl.port || getDefaultActionPort(specUrl.protocol); + if (clientExplicitPort !== specEffectivePort) { + return { + isValid: false, + message: `Port mismatch: Client provided '${clientProvidedDomain}', but spec uses effective port '${specEffectivePort}'`, + normalizedSpecDomain, + normalizedClientDomain, + }; + } + } + return { - isValid: false, - message: `Domain mismatch: Client provided '${clientProvidedDomain}', but spec uses '${specHostname}'`, + isValid: true, normalizedSpecDomain, normalizedClientDomain, };