refactor: route captain custom tool requests through SafeFetch (#14620)
Captain custom tools previously used their own hand-rolled `Net::HTTP` request code. This moves them onto `SafeFetch`, the same shared HTTP-fetching helper already used by webhooks, uploads, avatar imports, and the Captain page crawler. Behavior for normal tools is unchanged — they just now share one consistent path for host resolution, timeouts, response size limits, and redirect handling. **What changed** - `HttpTool#execute_http_request` now delegates to `SafeFetch.fetch` instead of building `Net::HTTP` requests by hand. Auth headers, basic auth, metadata headers, JSON content-type, and the 1 MB response cap all map onto `SafeFetch` options. - Removed ~80 lines of bespoke request/validation plumbing from `HttpTool`. - The custom tools `test` endpoint now reads the response body string directly (the executor returns the body rather than a response object). **How to test** 1. Enable `custom_tools` (or `captain_integration_v2`) for an account. 2. Create a Captain custom tool pointing at a public HTTPS endpoint (e.g. a test API). 3. Use the **Test** button in the tool form — you should get a success result. 4. Run the tool from a Captain conversation and confirm the response is returned/templated as before. **Gotchas** - **Local dev:** `SafeFetch` blocks requests to private/loopback addresses by default. If you're testing a custom tool against a service on `localhost` or a private IP during development, set `SAFE_FETCH_ALLOW_PRIVATE_NETWORK=true` or the request will be rejected. (This matches how the rest of `SafeFetch` already behaves locally.) - **Test endpoint status field:** on success the `test` response now reports `status: 200` rather than the exact 2xx code (201/204/etc.), because `SafeFetch` signals success-vs-failure rather than exposing the raw response. The UI only checks the 2xx range, so this is invisible there — but worth knowing if anything consumes the API directly. - **Non-2xx responses** still surface as an error (same as before), now via `SafeFetch::HttpError`.
This commit is contained in:
Shivam Mishra
@@ -27,8 +27,8 @@ class Api::V1::Accounts::Captain::CustomToolsController < Api::V1::Accounts::Bas
|
||||
|
||||
def test
|
||||
tool = account_custom_tools.new(custom_tool_params)
|
||||
result = execute_test_request(tool)
|
||||
render json: { status: result.code.to_i, body: result.body.to_s.truncate(500) }
|
||||
body = execute_test_request(tool)
|
||||
render json: { status: 200, body: body.to_s.truncate(500) }
|
||||
rescue StandardError => e
|
||||
render json: { error: e.message }, status: :unprocessable_content
|
||||
end
|
||||
|
||||
@@ -15,8 +15,8 @@ class Captain::Tools::HttpTool < Agents::Tool
|
||||
url = @custom_tool.build_request_url(params)
|
||||
body = @custom_tool.build_request_body(params)
|
||||
|
||||
response = execute_http_request(url, body, tool_context)
|
||||
@custom_tool.format_response(response.body)
|
||||
response_body = execute_http_request(url, body, tool_context)
|
||||
@custom_tool.format_response(response_body)
|
||||
rescue StandardError => e
|
||||
Rails.logger.error("HttpTool execution error for #{@custom_tool.slug}: #{e.class} - #{e.message}")
|
||||
'An error occurred while executing the request'
|
||||
@@ -24,89 +24,32 @@ class Captain::Tools::HttpTool < Agents::Tool
|
||||
|
||||
private
|
||||
|
||||
PRIVATE_IP_RANGES = [
|
||||
IPAddr.new('127.0.0.0/8'), # IPv4 Loopback
|
||||
IPAddr.new('10.0.0.0/8'), # IPv4 Private network
|
||||
IPAddr.new('172.16.0.0/12'), # IPv4 Private network
|
||||
IPAddr.new('192.168.0.0/16'), # IPv4 Private network
|
||||
IPAddr.new('169.254.0.0/16'), # IPv4 Link-local
|
||||
IPAddr.new('::1'), # IPv6 Loopback
|
||||
IPAddr.new('fc00::/7'), # IPv6 Unique local addresses
|
||||
IPAddr.new('fe80::/10') # IPv6 Link-local
|
||||
].freeze
|
||||
|
||||
# Limit response size to prevent memory exhaustion and match LLM token limits
|
||||
# 1MB of text ≈ 250K tokens, which exceeds most LLM context windows
|
||||
MAX_RESPONSE_SIZE = 1.megabyte
|
||||
|
||||
# Route through SafeFetch so custom tool requests share the app's centralized HTTP
|
||||
# fetching (resolution, timeouts, response size limits, and redirect handling).
|
||||
def execute_http_request(url, body, tool_context)
|
||||
uri = URI.parse(url)
|
||||
json_body = body if @custom_tool.http_method == 'POST'
|
||||
|
||||
# Check if resolved IP is private
|
||||
check_private_ip!(uri.host)
|
||||
|
||||
http = Net::HTTP.new(uri.host, uri.port)
|
||||
http.use_ssl = uri.scheme == 'https'
|
||||
http.read_timeout = 30
|
||||
http.open_timeout = 10
|
||||
http.max_retries = 0 # Disable redirects
|
||||
|
||||
request = build_http_request(uri, body)
|
||||
apply_authentication(request)
|
||||
apply_metadata_headers(request, tool_context)
|
||||
|
||||
response = http.request(request)
|
||||
|
||||
raise "HTTP request failed with status #{response.code}" unless response.is_a?(Net::HTTPSuccess)
|
||||
|
||||
validate_response!(response)
|
||||
|
||||
response
|
||||
response_body = +''
|
||||
SafeFetch.fetch(
|
||||
url,
|
||||
method: @custom_tool.http_method == 'POST' ? :post : :get,
|
||||
body: json_body,
|
||||
headers: request_headers(tool_context, json_body),
|
||||
http_basic_authentication: @custom_tool.build_basic_auth_credentials,
|
||||
max_bytes: MAX_RESPONSE_SIZE,
|
||||
validate_content_type: false
|
||||
) { |result| response_body = result.tempfile.read }
|
||||
response_body
|
||||
end
|
||||
|
||||
def check_private_ip!(hostname)
|
||||
ip_address = IPAddr.new(Resolv.getaddress(hostname))
|
||||
|
||||
raise 'Request blocked: hostname resolves to private IP address' if PRIVATE_IP_RANGES.any? { |range| range.include?(ip_address) }
|
||||
rescue Resolv::ResolvError, SocketError => e
|
||||
raise "DNS resolution failed: #{e.message}"
|
||||
end
|
||||
|
||||
def validate_response!(response)
|
||||
content_length = response['content-length']&.to_i
|
||||
if content_length && content_length > MAX_RESPONSE_SIZE
|
||||
raise "Response size #{content_length} bytes exceeds maximum allowed #{MAX_RESPONSE_SIZE} bytes"
|
||||
end
|
||||
|
||||
return unless response.body && response.body.bytesize > MAX_RESPONSE_SIZE
|
||||
|
||||
raise "Response body size #{response.body.bytesize} bytes exceeds maximum allowed #{MAX_RESPONSE_SIZE} bytes"
|
||||
end
|
||||
|
||||
def build_http_request(uri, body)
|
||||
if @custom_tool.http_method == 'POST'
|
||||
request = Net::HTTP::Post.new(uri.request_uri)
|
||||
if body
|
||||
request.body = body
|
||||
request['Content-Type'] = 'application/json'
|
||||
end
|
||||
else
|
||||
request = Net::HTTP::Get.new(uri.request_uri)
|
||||
end
|
||||
request
|
||||
end
|
||||
|
||||
def apply_authentication(request)
|
||||
def request_headers(tool_context, json_body)
|
||||
headers = @custom_tool.build_auth_headers
|
||||
headers.each { |key, value| request[key] = value }
|
||||
|
||||
credentials = @custom_tool.build_basic_auth_credentials
|
||||
request.basic_auth(*credentials) if credentials
|
||||
end
|
||||
|
||||
def apply_metadata_headers(request, tool_context)
|
||||
state = tool_context&.state || {}
|
||||
metadata_headers = @custom_tool.build_metadata_headers(state)
|
||||
metadata_headers.each { |key, value| request[key] = value }
|
||||
headers.merge!(@custom_tool.build_metadata_headers(tool_context&.state || {}))
|
||||
headers['Content-Type'] = 'application/json' if json_body.present?
|
||||
headers
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user