Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 34 additions & 11 deletions lib/vector_mcp/image_util.rb
Original file line number Diff line number Diff line change
Expand Up @@ -231,29 +231,52 @@ def determine_final_mime_type(explicit_mime_type, detected_mime_type)
# base_directory: "/app/uploads"
# )
def file_to_mcp_image_content(file_path, validate: true, max_size: DEFAULT_MAX_SIZE, base_directory: nil)
validate_path_safety!(file_path, base_directory) if base_directory
validated_path = validate_path_safety!(file_path, base_directory)

raise ArgumentError, "Image file not found: #{file_path}" unless File.exist?(file_path)
raise ArgumentError, "Image file not found: #{validated_path}" unless File.exist?(validated_path)

raise ArgumentError, "Image file not readable: #{file_path}" unless File.readable?(file_path)
raise ArgumentError, "Image file not readable: #{validated_path}" unless File.readable?(validated_path)

binary_data = File.binread(file_path)
binary_data = File.binread(validated_path)
to_mcp_image_content(binary_data, validate: validate, max_size: max_size)
end

# Validates that a file path does not escape the given base directory.
# Validates that a file path is safe to access.
#
# When +base_directory+ is provided, the resolved path must reside within it.
# When +base_directory+ is omitted, the path is still canonicalized and
# rejected if it contains path traversal sequences (+..+) that resolve
# outside the current working directory.
#
# @param file_path [String] The file path to validate.
# @param base_directory [String] The base directory boundary.
# @raise [ArgumentError] If the resolved path is outside base_directory.
# @param base_directory [String, nil] Optional base directory boundary.
# @return [String] The canonicalized, validated path.
# @raise [ArgumentError] If path traversal is detected.
# @api private
def validate_path_safety!(file_path, base_directory)
resolved_base = File.expand_path(base_directory)
resolved_path = File.expand_path(file_path, resolved_base)
if base_directory
resolved_base = File.expand_path(base_directory)
resolved_path = File.expand_path(file_path, resolved_base)

return if resolved_path.start_with?("#{resolved_base}/") || resolved_path == resolved_base
unless resolved_path.start_with?("#{resolved_base}/") || resolved_path == resolved_base
raise ArgumentError, "Path traversal detected: resolved path is outside the allowed base directory"
end
else
resolved_path = File.expand_path(file_path)

# Reject paths that use traversal sequences to escape upward, even without
# an explicit base directory. This catches the common case of
# user-supplied input like "../../etc/passwd".
if file_path.to_s.include?("..")
canonical_base = File.expand_path(".")
unless resolved_path.start_with?("#{canonical_base}/") || resolved_path == canonical_base
raise ArgumentError,
"Path traversal detected: '#{file_path}' resolves outside the working directory"
end
end
end

raise ArgumentError, "Path traversal detected: resolved path is outside the allowed base directory"
resolved_path
end

# Extracts image metadata from binary data.
Expand Down
37 changes: 33 additions & 4 deletions lib/vector_mcp/transport/http_stream.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,16 @@ class HttpStream
DEFAULT_EVENT_RETENTION = 100 # Keep last 100 events for resumability
DEFAULT_REQUEST_TIMEOUT = 30 # Default timeout for server-initiated requests

# Default allowed origins — restrict to localhost by default for security.
DEFAULT_ALLOWED_ORIGINS = %w[
http://localhost
https://localhost
http://127.0.0.1
https://127.0.0.1
http://[::1]
https://[::1]
].freeze

# Initializes a new HTTP Stream transport.
#
# @param server [VectorMCP::Server] The server instance that will handle messages
Expand All @@ -62,7 +72,8 @@ class HttpStream
# @option options [String] :path_prefix ("/mcp") The base path for HTTP endpoints
# @option options [Integer] :session_timeout (300) Session timeout in seconds
# @option options [Integer] :event_retention (100) Number of events to retain for resumability
# @option options [Array<String>] :allowed_origins (["*"]) List of allowed origins for CORS. Use ["*"] to allow all origins.
# @option options [Array<String>] :allowed_origins Allowed origins for CORS validation.
# Defaults to localhost origins only. Pass ["*"] to allow all origins (NOT recommended for production).
def initialize(server, options = {})
@server = server
@logger = server.logger
Expand Down Expand Up @@ -668,7 +679,13 @@ def valid_get_accept?(env)
accept.include?("text/event-stream") || accept.include?("*/*")
end

# Validates the Origin header for security
# Validates the Origin header for security.
#
# Matches are checked both exactly and as prefix (so that
# +http://localhost+ in the allowed list matches +http://localhost:3000+).
#
# Requests without an Origin header are allowed through because they
# originate from non-browser contexts (curl, server-to-server, etc.).
#
# @param env [Hash] The Rack environment
# @return [Boolean] True if origin is allowed, false otherwise
Expand All @@ -678,7 +695,9 @@ def valid_origin?(env)
origin = env["HTTP_ORIGIN"]
return true if origin.nil? # Allow requests without Origin header (e.g., server-to-server)

@allowed_origins.include?(origin)
@allowed_origins.any? do |allowed|
origin == allowed || origin.start_with?("#{allowed}:")
end
end

# Logging and error handling
Expand Down Expand Up @@ -874,7 +893,17 @@ def initialize_configuration(options)
@path_prefix = normalize_path_prefix(options[:path_prefix] || DEFAULT_PATH_PREFIX)
@session_timeout = options[:session_timeout] || DEFAULT_SESSION_TIMEOUT
@event_retention = options[:event_retention] || DEFAULT_EVENT_RETENTION
@allowed_origins = options[:allowed_origins] || ["*"]
@allowed_origins = options[:allowed_origins] || DEFAULT_ALLOWED_ORIGINS

warn_on_permissive_origins if @allowed_origins.include?("*")
end

# Logs a security warning when wildcard origin is configured.
def warn_on_permissive_origins
logger.warn do
"[SECURITY] allowed_origins includes '*', which permits cross-origin requests from any website. " \
"This is not recommended for production. Specify explicit origins instead."
end
end

# Initialize core HTTP stream components
Expand Down
15 changes: 14 additions & 1 deletion spec/vector_mcp/image_util_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,20 @@
end.to raise_error(ArgumentError, /Path traversal detected/)
end

it "allows access without base_directory (backward compatible)" do
it "allows access without base_directory for non-traversal paths" do
result = described_class.file_to_mcp_image_content(temp_file.path)
expect(result[:type]).to eq("image")
end
end

context "without base_directory (enforced traversal rejection)" do
it "rejects relative paths with .. that escape the working directory" do
expect do
described_class.file_to_mcp_image_content("../../etc/passwd")
end.to raise_error(ArgumentError, /Path traversal detected/)
end

it "allows absolute paths that do not contain .." do
result = described_class.file_to_mcp_image_content(temp_file.path)
expect(result[:type]).to eq("image")
end
Expand Down
38 changes: 36 additions & 2 deletions spec/vector_mcp/transport/http_stream_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -560,18 +560,52 @@
let(:restricted_transport) { described_class.new(mock_mcp_server, allowed_origins: ["https://example.com"]) }
let(:restricted_app) { restricted_transport }

context "with unrestricted origins (default)" do
context "with default origins (localhost only)" do
it "allows requests without Origin header" do
post "/mcp", { "jsonrpc" => "2.0", "method" => "ping" }.to_json,
valid_headers.merge("CONTENT_TYPE" => "application/json")

expect(last_response.status).to eq(200)
end

it "allows requests with any Origin header" do
it "allows requests from localhost origins" do
post "/mcp", { "jsonrpc" => "2.0", "method" => "ping" }.to_json,
valid_headers.merge("CONTENT_TYPE" => "application/json", "HTTP_ORIGIN" => "http://localhost:3000")

expect(last_response.status).to eq(200)
end

it "allows requests from 127.0.0.1 origins" do
post "/mcp", { "jsonrpc" => "2.0", "method" => "ping" }.to_json,
valid_headers.merge("CONTENT_TYPE" => "application/json", "HTTP_ORIGIN" => "http://127.0.0.1:8080")

expect(last_response.status).to eq(200)
end

it "rejects requests from non-localhost origins" do
post "/mcp", { "jsonrpc" => "2.0", "method" => "ping" }.to_json,
valid_headers.merge("CONTENT_TYPE" => "application/json", "HTTP_ORIGIN" => "https://malicious.com")

expect(last_response.status).to eq(403)
end
end

context "with wildcard origins (opt-in)" do
let(:wildcard_transport) do
described_class.new(mock_mcp_server, allowed_origins: ["*"])
end
let(:app) { wildcard_transport }

before do
allow(wildcard_transport.session_manager).to receive(:get_or_create_session).with(session_id, anything).and_return(mock_session)
allow(wildcard_transport.session_manager).to receive(:get_session).with(session_id).and_return(mock_session)
allow(mock_session_context).to receive(:request_context=)
end

it "allows requests from any origin" do
post "/mcp", { "jsonrpc" => "2.0", "method" => "ping" }.to_json,
valid_headers.merge("CONTENT_TYPE" => "application/json", "HTTP_ORIGIN" => "https://anywhere.com")

expect(last_response.status).to eq(200)
end
end
Expand Down
Loading