search

codacy / codacy-analysis-cli-action

GitHub Action for the codacy-analysis-cli
https://github.com/codacy/codacy-analysis-cli
Apache License 2.0
58 stars 18 forks source link

action: prevent globbing with double quotes #68

Closed ljmf00 closed 11 months ago

ljmf00 commented 2 years ago

This patch adds double quotes on variables to prevent globbing and prevent evaluation errors such as:

line 1: [: =: unary operator expected

It also add consistency on bash string equality comparison by using == operator instead of =.

Signed-off-by: Luís Ferreira contact@lsferreira.net

Since this is behaving "good" by accident, please double check if this doesn't break any specific use-case of this action. I recommend changing this to standalone scripts and passing GitHub variables by argument and parse them using something like POSIX getopt. See https://man7.org/linux/man-pages/man1/getopts.1p.html

ljmf00 commented 2 years ago

CC @lolgab

ale5000-git commented 2 years ago

@ljmf00: "==" is not POSIX compliant and also it won't make any difference in single brackets. Still to be usefull it should be inside [[ ]] instead of [ ]

ljmf00 commented 2 years ago

@ljmf00: "==" is not POSIX compliant and also it won't make any difference in single brackets. Still to be usefull it should be inside [[ ]] instead of [ ]

The usage of non-POSIX complaint features is not a problem here, since the action uses the bash shell, and that feature is specified in bash. I can't also see that as an argument since the existing code uses bash-specific features after all.

As the commit message suggested, these changes were to improve consistency in one way or another and not about the usefulness of equality/pattern matching in bash, as a feature. Arguing about the decision of making it consistent with == instead of = is that it makes it more readable/less strange among other languages syntax.

ljmf00 commented 1 year ago

68

What do you mean?