Skip to content

Rubocop redundant module info keys#21633

Merged
cgranleese-r7 merged 3 commits into
rapid7:masterfrom
sjanusz-r7:rubocop-redundant-module-info-keys
Jul 7, 2026
Merged

Rubocop redundant module info keys#21633
cgranleese-r7 merged 3 commits into
rapid7:masterfrom
sjanusz-r7:rubocop-redundant-module-info-keys

Conversation

@sjanusz-r7

@sjanusz-r7 sjanusz-r7 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Description

Closes #21606
This PR adds a RuboCop rule to help contributors with the redundant metadata that is often copy-pasted in the module metadata hash (arch and platform), if the Targets already has them defined.

Related work: #20786

This should follow the already-existing code conventions for other rubocop rules.

Potential scenarios

Some 'mock' module metadata, that you can store as a file and run rubocop against:

# frozen_string_literal: true

# BAD: Arch is redundant because all targets define it
class MetasploitModule < Msf::Exploit::Remote
  def initialize(info = {})
    super(
      update_info(
        info,
        'Name' => 'Bad Example - Redundant Arch',
        'Description' => %q{
          This fake module demonstrates a redundant top-level Arch.
          Every target already specifies Arch, so the top-level value is unnecessary.
        },
        'Author' => ['Test Author'],
        'License' => MSF_LICENSE,
        'Platform' => 'win',
        'Arch' => ARCH_X86,
        'Targets' => [
          ['Windows x86', { 'Arch' => ARCH_X86 }],
          ['Windows x64', { 'Arch' => ARCH_X64 }]
        ],
        'DefaultTarget' => 0,
        'DisclosureDate' => '2025-01-01',
        'Notes' => {
          'Stability' => [CRASH_SAFE],
          'SideEffects' => [IOC_IN_LOGS],
          'Reliability' => [REPEATABLE_SESSION]
        }
      )
    )
  end
end
# frozen_string_literal: true

# BAD: Both Arch and Platform are redundant because all targets define them
class MetasploitModule < Msf::Exploit::Remote
  def initialize(info = {})
    super(
      update_info(
        info,
        'Name' => 'Bad Example - Redundant Arch and Platform',
        'Description' => %q{
          This fake module demonstrates redundant top-level Arch AND Platform.
          Every target already specifies both, so neither is needed at the top level.
        },
        'Author' => ['Test Author'],
        'License' => MSF_LICENSE,
        'Platform' => 'win',
        'Arch' => [ARCH_X86, ARCH_X64],
        'Targets' => [
          ['Windows x86', { 'Platform' => 'win', 'Arch' => ARCH_X86 }],
          ['Windows x64', { 'Platform' => 'win', 'Arch' => ARCH_X64 }],
          ['Windows ARM', { 'Platform' => 'win', 'Arch' => ARCH_AARCH64 }]
        ],
        'DefaultTarget' => 0,
        'DisclosureDate' => '2025-01-01',
        'Notes' => {
          'Stability' => [CRASH_SAFE],
          'SideEffects' => [IOC_IN_LOGS],
          'Reliability' => [REPEATABLE_SESSION]
        }
      )
    )
  end
end
# frozen_string_literal: true

# BAD: Platform is redundant because all targets define it
class MetasploitModule < Msf::Exploit::Remote
  def initialize(info = {})
    super(
      update_info(
        info,
        'Name' => 'Bad Example - Redundant Platform',
        'Description' => %q{
          This fake module demonstrates a redundant top-level Platform.
          Every target already specifies Platform, so the top-level value is unnecessary.
        },
        'Author' => ['Test Author'],
        'License' => MSF_LICENSE,
        'Platform' => 'win',
        'Targets' => [
          ['Windows x86', { 'Platform' => 'win', 'Arch' => ARCH_X86 }],
          ['Windows x64', { 'Platform' => 'win', 'Arch' => ARCH_X64 }]
        ],
        'DefaultTarget' => 0,
        'DisclosureDate' => '2025-01-01',
        'Notes' => {
          'Stability' => [CRASH_SAFE],
          'SideEffects' => [IOC_IN_LOGS],
          'Reliability' => [REPEATABLE_SESSION]
        }
      )
    )
  end
end
# frozen_string_literal: true

# GOOD: Targets have empty hashes, so top-level Arch/Platform provide the values
class MetasploitModule < Msf::Exploit::Remote
  def initialize(info = {})
    super(
      update_info(
        info,
        'Name' => 'Good Example - Empty Target Hashes',
        'Description' => %q{
          This fake module has targets with empty option hashes. The top-level
          Arch and Platform are needed as the fallback for all targets.
        },
        'Author' => ['Test Author'],
        'License' => MSF_LICENSE,
        'Arch' => [ARCH_X86, ARCH_X64],
        'Platform' => 'win',
        'Targets' => [
          ['Automatic', {}],
          ['Manual', {}]
        ],
        'DefaultTarget' => 0,
        'DisclosureDate' => '2026-01-01',
        'Notes' => {
          'Stability' => [CRASH_SAFE],
          'SideEffects' => [IOC_IN_LOGS],
          'Reliability' => [REPEATABLE_SESSION]
        }
      )
    )
  end
end
# frozen_string_literal: true

# GOOD: No Targets defined, so top-level Arch/Platform are the only source of truth
class MetasploitModule < Msf::Exploit::Remote
  def initialize(info = {})
    super(
      update_info(
        info,
        'Name' => 'Good Example - No Targets',
        'Description' => %q{
          This fake module has no Targets at all, so the top-level Arch and
          Platform are required and should not trigger any offence.
        },
        'Author' => ['Test Author'],
        'License' => MSF_LICENSE,
        'Arch' => ARCH_X86,
        'Platform' => 'win',
        'DisclosureDate' => '2025-01-01',
        'Notes' => {
          'Stability' => [CRASH_SAFE],
          'SideEffects' => [IOC_IN_LOGS],
          'Reliability' => [REPEATABLE_SESSION]
        }
      )
    )
  end
end
# frozen_string_literal: true

# GOOD: Top-level Arch is necessary because not all targets define it
class MetasploitModule < Msf::Exploit::Remote
  def initialize(info = {})
    super(
      update_info(
        info,
        'Name' => 'Good Example - Targets Missing Arch',
        'Description' => %q{
          This fake module correctly uses a top-level Arch because one target
          does not override it. Framework falls back to the top-level value.
        },
        'Author' => ['Test Author'],
        'License' => MSF_LICENSE,
        'Arch' => ARCH_X86,
        'Platform' => 'win',
        'Targets' => [
          ['Windows x86', { 'Arch' => ARCH_X86 }],
          ['Automatic', {}]
        ],
        'DefaultTarget' => 0,
        'DisclosureDate' => '2026-01-01',
        'Notes' => {
          'Stability' => [CRASH_SAFE],
          'SideEffects' => [IOC_IN_LOGS],
          'Reliability' => [REPEATABLE_SESSION]
        }
      )
    )
  end
end

Breaking Changes

None

Reviewer Notes

Verification Steps

  • Run the rspec tests

Test Evidence

➜  metasploit-framework git:(rubocop-redundant-module-info-keys) ✗ bundle exec rspec spec/rubocop/cop/lint/module_redundant_arch_platform_spec.rb  
...
18 examples, 0 failures

AI Usage Disclosure

Claude was used.

Pre-Submission Checklist

  • Included a corresponding documentation markdown file in documentation/modules (new modules only)
  • No sensitive information (IP addresses, credentials, API keys, hashes) in code or documentation
  • Tested on the target environment specified in the Environment section above
  • Included RSpec tests for library changes (encouraged for lib/ changes)
  • Read the CONTRIBUTING.md and module acceptance guidelines

@sjanusz-r7 sjanusz-r7 marked this pull request as ready for review July 2, 2026 11:46
@github-project-automation github-project-automation Bot moved this from Todo to In Progress in Metasploit Kanban Jul 7, 2026
@cgranleese-r7 cgranleese-r7 self-assigned this Jul 7, 2026
@cgranleese-r7

cgranleese-r7 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Looks good to me 👍

Tests output:

Finished in 0.1734 seconds (files took 2.98 seconds to load)
18 examples, 0 failures

Example test file output:

❯ rubocop --only Lint/ModuleRedundantArchPlatform modules/rubocop_test_file.rb -A
WARN: Unresolved or ambiguous specs during Gem::Specification.reset:
      stringio (>= 0)
      Available/installed versions of this gem:
      - 3.1.7
      - 3.1.1
WARN: Clearing out unresolved specs. Try 'gem cleanup <gem>'
Please report a bug if this causes problems.
Inspecting 1 file
W

Offenses:

modules/rubocop_test_file.rb:17:9: W: [Corrected] Lint/ModuleRedundantArchPlatform: Remove top-level Arch as it is already defined in all Targets
        'Arch' => ARCH_X86,
        ^^^^^^
modules/rubocop_test_file.rb:48:9: W: [Corrected] Lint/ModuleRedundantArchPlatform: Remove top-level Platform as it is already defined in all Targets
        'Platform' => 'win',
        ^^^^^^^^^^
modules/rubocop_test_file.rb:49:9: W: [Corrected] Lint/ModuleRedundantArchPlatform: Remove top-level Arch as it is already defined in all Targets
        'Arch' => [ARCH_X86, ARCH_X64],
        ^^^^^^
modules/rubocop_test_file.rb:81:9: W: [Corrected] Lint/ModuleRedundantArchPlatform: Remove top-level Platform as it is already defined in all Targets
        'Platform' => 'win',
        ^^^^^^^^^^

1 file inspected, 4 offenses detected, 4 offenses corrected

@cgranleese-r7 cgranleese-r7 added the rn-no-release-notes no release notes label Jul 7, 2026
@cgranleese-r7 cgranleese-r7 merged commit dd081ab into rapid7:master Jul 7, 2026
20 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Metasploit Kanban Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rn-no-release-notes no release notes

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Rubocop rule to enforce module metadata conformity with targets

3 participants