Setting $home wrong can cause data loss, but managehome is required for SSH key setup to work, apparently.

[oranenj]

If managehome = true, the module will unconditionally remove the user's home directory. Unfortunately, there is no validation that the removed path makes sense at all; the module will happily rm -rf /, which might be catastrophic.

There also seems to be no way to opt out of the rm -rf behaviour; if you set managehome to false, ssh key management stops working (in fact, combined with purge_ssh_keys, turns out it will simply remove every managed user's ssh keys)

Something as simple as the following demonstrates the issue

class {"accounts":
    users          => {
        anuser     => {
            ensure => 'absent',
            home   => '/someimportantdata',
        }
    }
}

You could set home to /, but I don't have a disposable virtual machine to test that scenario. Obviously, this kind of a mistake would be catastrophic if you were to use this module to manage users across a large fleet of servers.

At the very least, the code ought to be much more paranoid about what it removes. I'd prefer it not run any such commands at all.

[oranenj]

As an addendum, the module does not quote the value of ${home_dir} when it runs the test and rm -rf commands, so there is a potential injection vulnerability.

It seems test -d / home/user fails, so a simple typo won't rm -rf /, but that's just luck, really.

[deric]

Would it be ok to remove home directory only $force_removal is set to true? (default would be false). In #23 we've introduced executing pkill which might be safer to disable by default.

[oranenj]

Kiling the user's processes is fine and even necessary IMO. It's the rm -rf that is scary. Hmm.

You should at least quote the commands properly, run the exec as the user you're removing (never as root), and whitelist path patterns eligible for autoremoval.

I'd also call the option something like destroy_homedir_on_remove to make it clear it destroys data.