Skip to content

Add MCP module notes and comments for hosts#21512

Open
adfoster-r7 wants to merge 1 commit into
rapid7:masterfrom
adfoster-r7:add-mcp-module-notes-and-comments-for-hosts
Open

Add MCP module notes and comments for hosts#21512
adfoster-r7 wants to merge 1 commit into
rapid7:masterfrom
adfoster-r7:add-mcp-module-notes-and-comments-for-hosts

Conversation

@adfoster-r7

Copy link
Copy Markdown
Contributor

Updates the Metasploit MCP tool to expose note information on Metasploit modules, as well as host comments.

Verification

Running the inspector tool:

npx @modelcontextprotocol/inspector -- bundle exec ./msfmcpd

Veriftying module notes:

image

Verifying host comments:

image


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

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'

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:

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

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) }

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'

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

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.

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])}..."

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rn-enhancement release notes enhancement

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

4 participants