Use Puma::Server instead of Puma::Launcher to fix Ctrl+C handling#21661
Use Puma::Server instead of Puma::Launcher to fix Ctrl+C handling#21661kx7m2qd wants to merge 2 commits into
Conversation
Puma::Launcher installs process-wide SIGINT/SIGTERM signal traps as part of its startup, intended for when Puma owns the entire process. Since the MCP server runs on a background thread inside msfconsole, these traps are process-global and silently override msfconsole's own Ctrl+C handling. Puma::Server is the lower-level, embeddable API and does not install any signal traps, making it safe to run inside a host process like msfconsole. Fixes rapid7#21650
dwelch-r7
left a comment
There was a problem hiding this comment.
Tested and seems to work well, one minor comment but once that's addressed I think this is good to go
| puma_config = Puma::Configuration.new do |config| | ||
| config.bind "tcp://#{bind_host}:#{port}" | ||
| config.threads min_threads, max_threads | ||
| config.workers workers |
There was a problem hiding this comment.
Does Puma::Launcher not have support for workers? if it does let's add it in, otherwise we should remove all references to workers if they aren't used anymore
There was a problem hiding this comment.
Good question turns out workers goes a bit deeper than server.rb. It's actually a supported, validated config option (MSF_MCP_WORKERS env var / mcp.workers in the config file, validated in config/validator.rb, loaded in config/loader.rb) that's wired through to the standalone msfmcpd binary via application.rb.
Since Puma::Server doesn't support clustering (that's Puma::Cluster, which Puma::Launcher used internally), workers is currently a no-op wherever it flows through start_http. Given the plugin path always uses the default (PUMA_WORKERS = 0), this PR's fix doesn't change plugin behavior either way but I don't want to silently strip a documented option that might affect the standalone msfmcpd binary without testing that path too.
Given the scope, I'd propose: keep workers in this PR as-is (still a no-op, same as before my change — not a regression I'm introducing), and I'll follow up with a separate PR/issue to properly deprecate or reintroduce cluster support for workers, after testing the msfmcpd binary path specifically. Let me know if you'd rather I just strip it now instead happy to do either, just flagging the wider surface first.
There was a problem hiding this comment.
The plugin only uses 0 sure but there are other ways of starting up the MCP server using msfmcpd where if you set MSF_MCP_WORKERS=4 it would work yes?
There was a problem hiding this comment.
You're right, confirmed MSF_MCP_WORKERS=4 via msfmcpd would genuinely lose clustering with this change, that's a real regression I missed since I only tested the plugin path (always workers: 0).
Digging into why: Puma::Launcher 's signal trapping is largely there to support clustering coordinating worker processes needs the parent to own SIGTERM/SIGINT. That's actually fine for msfmcpd (it's a standalone process, same as any CLI daemon), it's only a problem for the plugin case where we're embedded inside msfconsole.
Proposing a conditional: use Puma::Server when workers == 0 (the plugin's case, fixes the Ctrl+C bug), and keep Puma::Launcher when workers > 0 (preserves clustering for msfmcpd, and signal ownership there isn't an issue). Will push an update and test both paths (plugin + msfmcpd with MSF_MCP_WORKERS set) before pinging you again.
There was a problem hiding this comment.
Pushed the fix verified both paths:
Plugin (mcp start, workers=0): Ctrl+C still shows the normal Interrupt: use the 'exit' command to quit, and mcp stop/mcp restart work cleanly.
Standalone (MSF_MCP_WORKERS=2 ./msfmcpd): confirmed clustering is genuinely preserved —ps -ef --forestshows the parent Puma process with 2 real cluster worker child processes underneath it, same as before this change.
Confirmed with your exact example MSF_MCP_WORKERS=4 ./msfmcpd now spawns 4 real cluster workers:
puma 6.6.0 (tcp://localhost:3000) [metasploit-framework]
\_ msfrpcd
\_ puma: cluster worker 0: 151501
\_ puma: cluster worker 1: 151501
\_ puma: cluster worker 2: 151501
\_ puma: cluster worker 3: 151501
And the plugin path (workers=0) still fixes the original Ctrl+C issue
mcp start/Ctrl+C/mcp stop/mcp restart all confirmed working cleanly.
Puma::Server does not support clustering, so falling back to Puma::Launcher when workers > 0 preserves the existing MSF_MCP_WORKERS behavior for the standalone msfmcpd binary, while still using Puma::Server (no signal trapping) for the plugin's embedded case where workers defaults to 0.
Puma::Launcher installs process-wide SIGINT/SIGTERM signal traps as part of its startup, intended for when Puma owns the entire process (e.g. run standalone via the
pumaCLI). Since the MCP server runs on a background thread inside msfconsole (spawned viaframework.threads.spawn), these traps are process-global and silently override msfconsole's own Ctrl+C handling, which relies on Ruby's defaultInterruptbehavior.This swaps
Puma::LauncherforPuma::Server, the lower-level, embeddable API.Puma::Serverdoes not install any signal traps, making it safe to run inside a host process like msfconsole. Also updatedshutdownto call.stop(true)directly on the server for graceful shutdown.Related Issue:
Fixes #21650
Verification Steps:
mcpplugin, runmcp start, then press Ctrl+C. Expected: console printsInterrupt: use the 'exit' command to quit(matching normal msfconsole behavior), rather than doing nothing.mcp stop. Expected: printsMCP server stoppedcleanly with no hang or error.mcp startfollowed bymcp restart. Expected: server restarts cleanly without error.Test Evidence:
msf > load mcp
[] MCP plugin loaded. Use mcp start to start the server.
[] Successfully loaded plugin: mcp
msf > mcp start
[] Auto-started msgrpc - User: msf, Pass: [redacted]
[] MCP server started on localhost:3000 (transport: http)
msf > Interrupt: use the 'exit' command to quit
msf > mcp stop
[] MCP server stopped
msf > mcp start
[] MCP server started on localhost:3000 (transport: http)
msf > mcp restart
[*] MCP server started on localhost:3000 (transport: http)
Environment:
| Operating System | Ubuntu 24.04 |
| Target Software/Hardware | N/A |
AI Usage Disclosure:
Used AI to help identify the root cause and draft the fix. All testing and verification was done manually by me.
Pre-Submission Checklist:
Reviewer Notes
Wasn't able to get the full local RSpec suite passing due to an unrelated local Postgres/msfdb environment setup issue on my machine verified manually instead (see Test Evidence). Happy to dig further if CI surfaces anything.