fix brakeman warning about possible command injection

[d-m-u]

This line's in our brakeman ignore right now because we were supposed to get back to it later.

Confidence: Medium
Category: Command Injection
Check: Execute
Message: Possible command injection
Code: system("kill -WINCH #{`pgrep -P 1 httpd`.chomp.to_i}")
File: lib/miq_apache/control.rb
Line: 55

[d-m-u]

@miq-bot add_label security

[miq-bot]

@d-m-u Cannot apply the following label because they are not recognized:

• security

All labels for ManageIQ/manageiq: https://github.com/ManageIQ/manageiq/labels

[d-m-u]

phooey
@miq-bot add_label core/security
silly bot

[chessbyte]

Would AwesomeSpawn.run("kill", :params {:WINCH => pid}) make more sense to build the command line without the possibility of injection?

[Fryguy]

I agree with @chessbyte but I'm not sure why we are shelling out at all, when the Process class should do this for us:

Process.kill("WINCH", pid)

[miq-bot]

Checked commit d-m-u@14dab93 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. ⭐

[Fryguy]

Not sure we have specs on this. @carbonin can you review?

[jrafanie]

👍

[simaishi]

Jansa backport details:

$ git log -1
commit 2fcab30c77b9fc6c4321a7a0d1c11d33d7e992e2
Author: Nick Carboni <ncarboni@redhat.com>
Date:   Thu May 7 08:53:48 2020 -0400

    Merge pull request #20130 from d-m-u/use_awesome_spawn_for_apache_stop

    fix brakeman warning about possible command injection

    (cherry picked from commit 34bfb385e198264a3c5ee8fa06c42b7ea1e27838)