Skip to content

Add ModuleExecution and related data models#228

Open
cdelafuente-r7 wants to merge 3 commits into
rapid7:masterfrom
cdelafuente-r7:module-execution-data-models
Open

Add ModuleExecution and related data models#228
cdelafuente-r7 wants to merge 3 commits into
rapid7:masterfrom
cdelafuente-r7:module-execution-data-models

Conversation

@cdelafuente-r7

Copy link
Copy Markdown
Contributor

This adds ModuleExecution, ModuleExecutionError and ModuleExecutionEvent data models as part of the reporting refactor in Metasploit Framework.

(I'll update this description once the related PR in Framework is ready)

@cdelafuente-r7 cdelafuente-r7 force-pushed the module-execution-data-models branch from fd6f050 to a091880 Compare June 16, 2026 16:19
@cdelafuente-r7 cdelafuente-r7 marked this pull request as ready for review June 16, 2026 16:19
KINDS = %w[run check import direct_write].freeze

# Module types as exposed by Framework.
MODULE_TYPES = %w[exploit auxiliary post payload encoder evasion nop external].freeze

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't think "external" is a module type in the same sense as the others. You can have for example an external exploit or auxiliary module that's implemented in Python. I would suggest removing it or tracking it as a flag so you can note something is an external-exploit, external-auxiliary, etc.

Comment on lines +23 to +25
# Terminal lifecycle states. `running` is the only non-terminal value
# and is permitted while {#ended_at} is `NULL`.
TERMINAL_STATUSES = %w[running success neutral expected_failure unhandled_exception].freeze

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If unhandled_exception is the inverse of expected_failure, it might be more intuitive to track it as unexpected_failure.

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.

Let me give more details about these status:

  • expected_failure: the module explicitly declared failure via fail_with. It is an error the module author has anticipated with a proper failure_reason.
  • unhandled_exception: a Ruby exception that escaped the module's rescue chain (NoMethodError, Rex::ConnectionError outside a rescue, etc.). The author did not anticipate this; there's an exception_class and backtrace.

So, unhandled_exception and expected_failure aren't strict inverses. For example, unhandled_exception will help answer question like "which modules are buggy?"

Comment thread app/models/mdm/module_execution.rb Outdated
MODULE_TYPES = %w[exploit auxiliary post payload encoder evasion nop external].freeze

# User interface or programmatic surface that initiated this execution.
ORIGINATING_UIS = %w[console rpc json_rpc mcp external import plugin autocheck].freeze

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
ORIGINATING_UIS = %w[console rpc json_rpc mcp external import plugin autocheck].freeze
ORIGINATORS = %w[console rpc json_rpc mcp external import plugin autocheck module].freeze

I'd suggest adding 'module' as an originator to account for modules running other modules. Additionally since some of these aren't exactly user-interfaces, it may be better to use a slightly more generic name like ORIGINATORS vs ORIGINATING_UIS.

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.

You're right it's not accurate. I would prefer using ORIGINATING_INTERFACES, which is more programmatic. ORIGINATORS sounds like a user, which seems slighly wrong with things like autocheck. I'm also fine if you disagree, I'll change it to ORIGINATORS, no problem.

Comment on lines +45 to +48
# @!attribute [rw] updated_at
# Last time this event row was updated.
#
# @return [DateTime]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Events should be immutable, so let's remove the updated_at column so once it's created, it stays the same.

Suggested change
# @!attribute [rw] updated_at
# Last time this event row was updated.
#
# @return [DateTime]

Comment on lines +75 to +79
# @!attribute [rw] updated_at
# Last time this error row was updated.
#
# @return [DateTime]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This should also be immutable.

Suggested change
# @!attribute [rw] updated_at
# Last time this error row was updated.
#
# @return [DateTime]

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.

Yes, this makes no sense for Error and Events. I removed these fields. Thanks!

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

Projects

Status: Ready

Development

Successfully merging this pull request may close these issues.

2 participants