Add rubocop rule for redeclaring pre-existing options

[bwatters-r7]

Description

This adds rubocop rules to catch when a contributor redeclares a pre-existing option and guides them to simply change that option's default value.
It still allows contributors to deregister and then re-register options that are different, but have the same key value.

@jheysel-r7 has caught me missing this four times in the last week. I figure it should be a rubocop rule, then I won't look as bad.

Related Issue:

Breaking Changes

None

Reviewer Notes

Files that break this rule so you can test:

• RPORT: modules/auxiliary/admin/appletv/appletv_display_image.rb:36
• SSL: modules/auxiliary/admin/http/foreman_openstack_satellite_priv_esc.rb:40
• RHOST: modules/auxiliary/client/hwbridge/connect.rb:48
• VHOST: modules/exploits/unix/webapp/opennetadmin_ping_cmd_injection.rb:50
• THREADS: modules/auxiliary/scanner/nessus/nessus_xmlrpc_ping.rb:30

Verification Steps

Run rubocop on the problematic files to verify it works.

Test Evidence

RuboCop reported 8 offenses across the 5 files.

1. appletv_display_image.rb

Opt::RPORT(7000)
^^^^^^^^^^^^^^^^ Lint/ModuleDuplicateOption

Correction:

'DefaultOptions' => {
'HttpUsername' => 'AirPlay',
'RPORT' => 7000
}

Remove Opt::RPORT(7000) from register_options.

2. foreman_openstack_satellite_priv_esc.rb

Opt::RPORT(443)
^^^^^^^^^^^^^^^
OptBool.new('SSL', [true, 'Use SSL', true])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Correction:

'DefaultOptions' => {
'RPORT' => 443,
'SSL' => true
}

Remove both registrations.

3. hwbridge/connect.rb

Opt::RPORT(8080)
^^^^^^^^^^^^^^^^
Opt::RHOST('127.0.0.1')
^^^^^^^^^^^^^^^^^^^^^^^

Correction:

'DefaultOptions' => {
'RHOST' => '127.0.0.1',
'RPORT' => 8080
}

Remove both registrations.

4. opennetadmin_ping_cmd_injection.rb

OptString.new('VHOST', [false, 'HTTP server virtual host'])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This does not change the default, so the correction is simply to remove the redundant registration. No DefaultOptions entry is
needed.

5. nessus_xmlrpc_ping.rb

Opt::RPORT(8834)
^^^^^^^^^^^^^^^^
OptInt.new('THREADS', [true, "The number of concurrent threads", 25])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Correction:

'DefaultOptions' => {
'SSL' => true,
'RPORT' => 8834,
'THREADS' => 25
}

AI Usage Disclosure

gpt-5.6 generated the rules and tested them

[jheysel-r7]

Will this allow module authors to re-register the same datastore option but with a different description? ie. a description that is more specific and tailored to the application the module is exploiting? I feel like that is a common and legit use case to reregister an option:

Does this cover that use case?

It still allows contributors to deregister and then re-register options that are different, but have the same key value.

[bwatters-r7]

Will this allow module authors to re-register the same datastore option but with a different description? ie. a description that is more specific and tailored to the application the module is exploiting? I feel like that is a common and legit use case to reregister an option:

Yes, but they will have to deregister it first, I believe. Kind of explicitly saying "No, I want it the other way"

[jheysel-r7]

Will this allow module authors to re-register the same datastore option but with a different description? ie. a description that is more specific and tailored to the application the module is exploiting? I feel like that is a common and legit use case to reregister an option:

Yes, but they will have to deregister it first, I believe. Kind of explicitly saying "No, I want it the other way"

Hmm, I'm not a huge fan of requiring the author to deregister the option in order to change the description. Can the rubocop rule only fire if the description of the re-registered option is identical to the original or if the option is being re-registered with no description?

[bwatters-r7]

Hmm, I'm not a huge fan of requiring the author to deregister the option in order to change the description. Can the rubocop rule only fire if the description of the re-registered option is identical to the original or if the option is being re-registered with no description?

Deregistering is easy. It would be as simple as:

deregister_options('TARGET_URI')
register options([
OptString.new('TARGET_URI', [ false, 'New URI Label', nil])
])

[jheysel-r7]

Deregistering is easy. It would be as simple as:

deregister_options('TARGET_URI')
register options([
OptString.new('TARGET_URI', [ false, 'New URI Label', nil])
])

I agree deregistering the datastore is easy, but I would also say it's unintuitive to require module authors to deregister the datastore option if they want change the description.

Its very common for authors to re-register for example the TARGETURI option to give a more detailed description of what URI the option should point to, 947 different modules do this (they would all now fail this rule):

➜  modules git:(feat/mod/simplehelp_oidc_rce) ✗ rg -l "OptString.new\('TARGETURI" | wc -l
     947

I feel like it would take much less effort to handle this on the rubocop side than to re-educate the whole committer base on how we now have to deregister the option before updating the description.

[dwelch-r7]

yea I have to agree with @jheysel-r7 here, I don't think users needing to know they need to de-register to re-register an option is a far expectation when it's already a common pattern to just re-register to update the description