Metasploit-framework: Suggestion: remove "Actions" from module with only one action (which is also the default one) to prevent confusion

[cnotin]

Steps to reproduce

1. use auxiliary/server/dhcp
2. show actions

Expected behavior

There shouldn't be any "action" for module with only one action (which is also the default one)

Current behavior

We see the "Service" action but... What does it do? Should I set it? Why is there only one? I think it's confusing...

msf5 auxiliary(server/dhcp) > show actions

Auxiliary actions:

   Name     Description
   ----     -----------
   Service  

Here is the action creation:
https://github.com/rapid7/metasploit-framework/blob/4a853beb8d8be4369cdef67d3d655385d1edfa00/modules/auxiliary/server/dhcp.rb#L20-L28

But it doesn't seem to be used afterwards.

Several other modules are concerned (non-exhaustive list), for example "ftp" or "fakedns" under auxiliary/server

Let me know what you think, and if you like it I can propose a PR to clean it.

[wvu-r7]

It should have a Description hash set.

[cnotin]

Yes that's another possibility. But even when there's only one action? Is it the chosen convention?

[wvu-r7]

I mean it already has that hash, and it should be set to describe the action.

[wvu-r7]

Even if you have only one option, it is useful to describe what you are going to do in the module. Removing it unconditionally sounds like a bad idea.

[wvu-r7]

Service is just poorly named and described.

[cnotin]

Ok I can try to add description to other modules too. I've seen a couple with a single action and no description

[wvu-r7]

Yeah, that's just bad practice.

[wvu-r7]

https://github.com/rapid7/metasploit-framework/blob/01a220ec21de4fab71bc4f6a28ebdaeed6843a75/modules/auxiliary/gather/vmware_vcenter_vmdir_ldap.rb#L31-L33

For instance, I find the description here useful. That said, I probably should have expanded it more. The implication is you're dumping from vmdir, but that could definitely be explicit, since not everyone reads the module description.

[cnotin]

Sure :)
I wasn't sure if the convention was to have "Actions" defined only when there were several possible (hence the suggestion to remove), or in all cases. Fine then ;)

[cnotin]

I'll propose a PR later to add better descriptions for those modules

[wvu-r7]

If you don't define it, there will be no listed action. Kind of a missed opportunity to be descriptive in the action you will perform. That's all. :)

[wvu-r7]

So maybe we can hide the table if there are no actions, but I prefer to add one if there isn't one.

[wvu-r7]

https://github.com/rapid7/metasploit-framework/pull/13301/files#r419855409

[wvu-r7]

Oh, nvm, it's already hidden if there's none.