Consistency in Post modules and Post module libraries

[bwatters-r7]

This is blatantly copy/pasted from #17110 (comment).

I think the PR itself is ready to land, but @bcoles hit at the heart of a challenge we face complete with suggestions for how we address the challenge, and I think we should keep tracking that outside the PR:

---

Off topic:

so my worry is that if we add these checks to everything, any module that does a chain of calls such as file? readable? setuid? copy will end up calling file? a ton of times. I think it should be checked once then done with...

I agree with not wanting to call file? a bunch of times and certainly don't like executing cmd_exec more than necessary. We don't have clear design guidelines for this. It is a balance between clean/consistent code and decreasing on-target activity.

Well outside the scope of this PR, but using this opportunity to bring it up and ramble about it.

This discussion is part of a larger issue due to organic growth of Post libraries from multiple different contributors spanning over a decade with no design guidelines. In short, modules and libraries using Post mixins don't know if they're going to receive an error result (nil / false) or a raised error. Having to handle both (sometimes but not others) creates unintuitive messy code and is often overlooked entirely. Also whether the library [v]prints an error message upon error is also extremely inconsistent.

Currently, it is common for a Post method to handle shell, powershell and meterpreter sessions, then return false or nil upon failure or error for powershell/shell sessions, but maybe also a raise upon error for Meterpreter sessions. This code pattern is common, but not universal. Similarly, raising is not consistent as only some error conditions result in a raised error.

Ensuring consistency across the entire codebase requires applying one of a few approaches:

1. For all Post methods, implement identical functionality for all session types (where possible) within each method. This will require expanding code for shell/powershell sessions to perform additional error checking. This offers consistency and granular error messages at the expense of significantly increasing the number of library calls (cmd_exec calls under the hood). Even then, it may not always be possible to achieve one-for-one parity with Meterpreter functionality.

• As a simple hypothetical example, a readable? Post method for a Meterpreter session may raise if the file does not exist, but for shell sessions it will return false. To ensure one-for-one parity the code path for shell session would also need to raise if the file doesn't exist by first calling file_exists? (resulting in additional cmd_exec calls).

2. Never raise from Post libraries. Instead, catch raised errors within the Post method and ensure false or nil is always returned (depending on whether the method is expected to return a Boolean or a value respectively). Ensure every use of these methods always checks the response for an error result.

3. Implement <method>_with_error equivalents of methods which can raise to be used in instances where the caller needs to know why something failed.

None of these approaches are particularly appealing and likely requires significant re-work of existing modules. There's no clear answer, but developing some design guidelines which ensure consistency of inconsistency would help.