priv-kweihmann / systemdlint

Systemd Linter
BSD 2-Clause "Simplified" License
32 stars 7 forks source link

Fix ExecValue prefix check #68

Closed zgauci closed 5 months ago

zgauci commented 5 months ago

The systemdlint tool's NoFailureCheck/ErrorCommandCouldFail triggers false-positives on the AdditionalErrors function of ExecValue. The function leverages the parent class version which checks if a character is contained within the field's value. However, the relevant characters only matter if they are a prefix in this case, not simply contained. As such, there are false positives for ExecStart* commands which contain a - in the command (or in the arguments supplied to it).

By providing a bit more flexibility to the check, we can provide an argument to determine what type of string search to use which allows the ExecValue to perform a prefix check instead of just within. All other Value classes should be unaffected and all ExecValue functions should care exclusively about prefixes. I have tested locally with a yocto build and some unit files that were showing false positives.

priv-kweihmann commented 5 months ago

@zgauci do you have an example at hand that could be added to the tests? So far the PR looks reasonable, even if I'm not a big fan of getattr, but a real example would be the icing on the cake

zgauci commented 5 months ago

I added a couple example unit files that trigger false-positives on master (ie. the good shows NoFailureCheck errors). I also updated the code to use lambda functions instead of the gettattr & __contains__ to try to feel a little less dirty about the changes. Let me know if there's anything else you'd like to see from this PR.