Skip to content

Add MCP module notes and comments for hosts #21512

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
adfoster-r7 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add MCP module notes and comments for hosts #21512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions docs/metasploit-framework.wiki/How-to-use-Metasploit-MCP-Server.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,9 @@ The [MCP Inspector](https://github.com/modelcontextprotocol/inspector) is an int

```
npx @modelcontextprotocol/inspector

# stdio transport type usage:
npx @modelcontextprotocol/inspector -- bundle exec ./msfmcpd
```

## Troubleshooting
Expand Down
7 changes: 5 additions & 2 deletions lib/msf/core/mcp/application.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ def run
authenticate_metasploit
initialize_mcp_server
start_mcp_server
rescue Interrupt
shutdown('INT')
rescue Msf::MCP::Config::ValidationError, Msf::MCP::Config::ConfigurationError => e
handle_configuration_error(e)
rescue Msf::MCP::Metasploit::ConnectionError => e
Expand Down Expand Up @@ -164,8 +166,9 @@ def initialize_logger
#
# @return [void]
def install_signal_handlers
Signal.trap('INT') { shutdown('INT'); exit 0 }
Signal.trap('TERM') { shutdown('TERM'); exit 0 }
@main_thread = Thread.current
Signal.trap('INT') { @main_thread.raise(Interrupt) }
Signal.trap('TERM') { @main_thread.raise(Interrupt) }

@adfoster-r7 adfoster-r7 May 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ctrl+c didn't work:

Server ready - waiting for MCP requests
Press Ctrl+C to shutdown
^CFatal error: can't be called from trap context
/Users/user/Documents/code/metasploit-framework/lib/rex/logging/log_dispatcher.rb:90:in `synchronize'
/Users/user/Documents/code/metasploit-framework/lib/rex/logging/log_dispatcher.rb:90:in `log'
/Users/user/Documents/code/metasploit-framework/lib/rex/logging/log_dispatcher.rb:182:in `ilog'
/Users/user/Documents/code/metasploit-framework/lib/msf/core/mcp/application.rb:72:in `shutdown'
/Users/user/Documents/code/metasploit-framework/lib/msf/core/mcp/application.rb:167:in `block in install_signal_handlers'

@cdelafuente-r7 cdelafuente-r7 Jun 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I traced back this issue and it is related to the call to ilog in shutdown, which call Mutex#synchronize at some point. This comes from the last change that has been made to make it compatible with Framework logging system. Mutex#synchronize can't be called from a signal trap context.
That said, during my testing, I found out @main_thread.raise doesn't work neither. The rescue Interrupt is never triggered in #run. However using a fresh new Thread seems to work:

Thread.new { shutdown('INT'); exit 0 }.join

Another option would be to use at_exit, which is what @dwelch-r7 suggests in this PR

This solution is simpler but we lose the ability to distinguish signals from normal exits. That said, since The Interrupt exception handler call shutdown('INT') even if a SIGTERM is received, so it will always logged as SIGINT. Maybe passing the signal in the Interrupt exception and then passing it to shutdown would solve this, but it is not a big deal.

Whatever solution we choose, we should add specs that check if #shutdown is correctly called when a INT or TERM signal is received.

Thought?

@cdelafuente-r7 cdelafuente-r7 Jun 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the issue with the Signal trap is that it gets overwritten by Puma when the http transport is used (see my comment here). So it looks like we haven't a lot of options.

@cdelafuente-r7 cdelafuente-r7 Jun 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dwelch-r7 's PR has landed now. We might want to rebase and check if this fixed this issue for you too.

end

# Load configuration from file or use defaults
Expand Down
2 changes: 1 addition & 1 deletion lib/msf/core/mcp/config/loader.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def self.apply_defaults(config)
config[:logging] ||= {}

config[:msf_api][:type] ||= 'messagepack'
config[:msf_api][:host] ||= 'localhost'
config[:msf_api][:host] ||= '127.0.0.1'

@adfoster-r7 adfoster-r7 May 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Binding to localhost would resolve to ipv6 then have issues:

$ ./msfmcpd
[DEPRECATION NOTICE] json-schema support for MultiJSON is deprecated and will be removed in a future version. To stop using MultiJSON, add `JSON::Validator.use_multi_json = false` to your application's initialization code.
No configuration file specified, using defaults
Validating configuration...
Configuration valid
Fatal error: Address family not supported by protocol family - connect(2) for [::1]:55553
/Users/user/.rvm/gems/ruby-3.2.5/gems/rex-socket-0.1.65/lib/rex/socket/comm/local.rb:267:in `connect'
/Users/user/.rvm/gems/ruby-3.2.5/gems/rex-socket-0.1.65/lib/rex/socket/comm/local.rb:267:in `block in create_by_type'
/Users/user/.rvm/gems/ruby-3.2.5/gems/timeout-0.4.3/lib/timeout.rb:185:in `block in timeout'
/Users/user/.rvm/gems/ruby-3.2.5/gems/timeout-0.4.3/lib/timeout.rb:38:in `handle_timeout'
/Users/user/.rvm/gems/ruby-3.2.5/gems/timeout-0.4.3/lib/timeout.rb:194:in `timeout'

@cdelafuente-r7 cdelafuente-r7 Jun 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking, did you get the same error with the MCP server, which also default to localhost?

Also, can you also update the example config files:

@dwelch-r7 dwelch-r7 Jun 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also ran into this error with the mcp server FWIW

@cdelafuente-r7 cdelafuente-r7 Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😞 Do you think this should also be updated?

config[:msf_api][:port] ||= (config[:msf_api][:type] == 'json-rpc') ? 8081 : 55553

config[:msf_api][:ssl] = config[:msf_api].fetch(:ssl, true)
Expand Down
4 changes: 3 additions & 1 deletion lib/msf/core/mcp/metasploit/response_transformer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def self.transform_module_info(info)
stance: info['stance'],
actions: info['actions'],
default_action: info['default_action'],
notes: info['notes'],
# TODO: write transformer for options
options: info['options']
}.compact
Expand All @@ -76,7 +77,8 @@ def self.transform_hosts(response)
os_language: host['os_lang'],
updated_at: format_timestamp(host['updated_at']),
purpose: host['purpose'],
info: host['info']
info: host['info'],
comments: host['comments']
}.compact
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/msf/core/mcp/rpc_manager.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def wait_for_rpc(timeout: DEFAULT_WAIT_TIMEOUT, interval: DEFAULT_WAIT_INTERVAL)
"Timed out waiting for RPC server after #{timeout} seconds"
end

@output.puts 'Waiting for RPC server to become available...'
@output.puts "Waiting for RPC server to become available at #{Rex::Socket.to_authority(@config[:msf_api][:host], @config[:msf_api][:port])}..."

@adfoster-r7 adfoster-r7 May 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding where it's connecting to:

  RPC server started via msfrpcd (PID: 37410)
- Waiting for RPC server to become available...
+ Waiting for RPC server to become available at 127.0.0.1:55553...

sleep(interval)
end
end
Expand Down
3 changes: 2 additions & 1 deletion lib/msf/core/mcp/tools/host_info.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ class HostInfo < ::MCP::Tool
os_language: { type: 'string' },
updated_at: { type: 'string' },
purpose: { type: 'string' },
info: { type: 'string' }
info: { type: 'string' },
comments: { type: 'string' }
}
}
}
Expand Down
1 change: 1 addition & 0 deletions lib/msf/core/mcp/tools/module_info.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class ModuleInfo < ::MCP::Tool
privileged: { type: 'boolean' },
has_check_method: { type: 'boolean' },
default_options: { type: 'object' },
notes: { type: 'object' },
references: { type: 'array', items: { type: ['string', 'object'] } },
targets: { type: 'object' },
default_target: { type: 'integer' },
Expand Down
4 changes: 2 additions & 2 deletions spec/lib/msf/core/mcp/config/loader_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,9 @@
expect(config[:msf_api][:type]).to eq('messagepack')
end

it 'sets default host to localhost' do
it 'sets default host to 127.0.0.1' do
config = described_class.load_from_hash(config_hash)
expect(config[:msf_api][:host]).to eq('localhost')
expect(config[:msf_api][:host]).to eq('127.0.0.1')
end

it 'sets default port to 55553 for messagepack' do
Expand Down
6 changes: 5 additions & 1 deletion spec/lib/msf/core/mcp/metasploit/response_transformer_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
'privileged' => true,
'check' => true,
'default_options' => { 'Option1' => 'Value1' },
'notes' => { 'SideEffects' => ['IOC_IN_LOGS'], 'Stability' => ['CRASH_SAFE'] },
'references' => [{'CVE' => '2017-0144'}, {'URL' => 'https://example.com'}],
'targets' => { 0 => 'Windows 7', 1 => 'Windows 8' },
'default_target' => 0,
Expand Down Expand Up @@ -99,6 +100,7 @@
privileged: true,
has_check_method: true,
default_options: { 'Option1' => 'Value1' },
notes: { 'SideEffects' => ['IOC_IN_LOGS'], 'Stability' => ['CRASH_SAFE'] },
references: [{'CVE' => '2017-0144'}, {'URL' => 'https://example.com'}],
targets: { 0 => 'Windows 7', 1 => 'Windows 8' },
default_target: 0,
Expand Down Expand Up @@ -173,6 +175,7 @@
'os_lang' => 'English',
'purpose' => 'server',
'info' => 'Web server',
'comments' => 'Primary web server',
'state' => 'alive',
'created_at' => 1609459200,
'updated_at' => 1640995200
Expand All @@ -192,7 +195,8 @@
hostname: 'testhost',
os_name: 'Linux',
os_flavor: 'Ubuntu',
state: 'alive'
state: 'alive',
comments: 'Primary web server'
)
expect(result[0][:created_at]).to eq('2021-01-01T00:00:00Z')
expect(result[0][:updated_at]).to eq('2022-01-01T00:00:00Z')
Expand Down
Loading