From 3dfb5061e13395e290625a9d9b2f60d78818e92b Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Wed, 10 Jun 2026 14:01:30 +0530 Subject: [PATCH] refactor: route captain custom tool requests through SafeFetch (#14620) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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`. --- .../captain/custom_tools_controller.rb | 4 +- enterprise/lib/captain/tools/http_tool.rb | 97 ++++--------------- 2 files changed, 22 insertions(+), 79 deletions(-) diff --git a/enterprise/app/controllers/api/v1/accounts/captain/custom_tools_controller.rb b/enterprise/app/controllers/api/v1/accounts/captain/custom_tools_controller.rb index fab952960..874197870 100644 --- a/enterprise/app/controllers/api/v1/accounts/captain/custom_tools_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/captain/custom_tools_controller.rb @@ -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 diff --git a/enterprise/lib/captain/tools/http_tool.rb b/enterprise/lib/captain/tools/http_tool.rb index b4593d27f..18ebcf53a 100644 --- a/enterprise/lib/captain/tools/http_tool.rb +++ b/enterprise/lib/captain/tools/http_tool.rb @@ -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