selinux_build_module_simple.sh: improve quoting

[kenyon]

If module_name or module_dir had a space, this script would fail.

Also avoid existence test for the tmp dir and use mkdir -p instead.

[smortex]

That look more correct!

I guess the CI don't the case with spaces but maybe you have a real-case scenario you can check against this branch to ensure correctness?

LGTM!

[kenyon]

Yeah, that's another problem with using a shell script like this, it would have to be tested in acceptance tests.

I found more command lines where filenames should be quoted. Tomorrow I'll see if I can add an acceptance test that fails without this PR.

[smortex]

Thinking about it, the "best" is probably to rely on shellquote to escape shell special chars. I can't really think that somebody able to inject malicious subcommands in a control-repo without validation has no prior root access to the system, but for the sake of best practice it may make sense to prevent such misuse.

[kenyon]

Thinking about it, the "best" is probably to rely on shellquote to escape shell special chars. I can't really think that somebody able to inject malicious subcommands in a control-repo without validation has no prior root access to the system, but for the sake of best practice it may make sense to prevent such misuse.

Hmmmm, the command line is actually already shellquoted:

puppet-selinux/manifests/module.pp

Line 83 in 37e1761

'simple' => shellquote($selinux::build::module_build_simple, $title, $module_dir),

I used a $title with a space in it, which is how I discovered this problem. So the shellquoting protects the exec command line, but I guess the problem is that usage of those arguments inside the script still needs proper quoting. What do you think?