Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use shellcheck for better compatibility in mde_installer.sh #67

Open
SvenMarquardt5772 opened this issue Dec 7, 2022 · 4 comments
Open

Comments

@SvenMarquardt5772
Copy link

I would suggest to use https://www.shellcheck.net/, to verify that the script works in more environments and also to have some sort of static code analysis.

Here is the current output of shellcheck.

In mde_installer.sh line 64:
    msg="${@:3}"
        ^------^ SC2124: Assigning an array to a string! Assign as array, or use * instead of @ to concatenate.


In mde_installer.sh line 101:
    if [ -n $DEBUG ]; then
            ^----^ SC2070: -n doesn't work with unquoted arguments. Quote or use [[ ]].


In mde_installer.sh line 117:
	    exit $2
                 ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	    exit "$2"


In mde_installer.sh line 122:
   if which python3 &> /dev/null; then
      ^---^ SC2230: which is non-standard. Use builtin 'command -v' instead.


In mde_installer.sh line 124:
   elif which python2 &> /dev/null; then
        ^---^ SC2230: which is non-standard. Use builtin 'command -v' instead.


In mde_installer.sh line 133:
   cat <<EOF | /usr/bin/env $(get_python)
                            ^-----------^ SC2046: Quote this to prevent word splitting.


In mde_installer.sh line 172:
    echo $proxy_params
         ^-----------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
    echo "$proxy_params"


In mde_installer.sh line 177:
    if [ -z $(which mdatp) ]; then
            ^------------^ SC2046: Quote this to prevent word splitting.
              ^---^ SC2230: which is non-standard. Use builtin 'command -v' instead.


In mde_installer.sh line 200:
    local out=$(eval $1 2>&1; echo "$?")
          ^-^ SC2155: Declare and assign separately to avoid masking return values.
                     ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
    local out=$(eval "$1" 2>&1; echo "$?")


In mde_installer.sh line 201:
    local exit_code=$(echo "$out" | tail -n1)
          ^-------^ SC2155: Declare and assign separately to avoid masking return values.


In mde_installer.sh line 208:
        if [ -n $DEBUG ]; then
                ^----^ SC2070: -n doesn't work with unquoted arguments. Quote or use [[ ]].


In mde_installer.sh line 215:
            log_error $2
                      ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
            log_error "$2"


In mde_installer.sh line 221:
    return $exit_code
           ^--------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
    return "$exit_code"


In mde_installer.sh line 237:
    while [ $retries -gt 0 ]
            ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
    while [ "$retries" -gt 0 ]


In mde_installer.sh line 249:
            log_info "[r] $(($1-$retries))/$1"
                                ^------^ SC2004: $/${} is unnecessary on arithmetic variables.


In mde_installer.sh line 266:
        . /etc/os-release
          ^-------------^ SC1091: Not following: /etc/os-release was not specified as input (see shellcheck -x).


In mde_installer.sh line 273:
        elif [[ $(grep -o -i "Red\ Hat" /etc/redhat-release) ]]; then
                ^-- SC2143: Use grep -q instead of comparing output with [ -n .. ].


In mde_installer.sh line 275:
        elif [[ $(grep -o -i "Centos" /etc/redhat-release) ]]; then
                ^-- SC2143: Use grep -q instead of comparing output with [ -n .. ].


In mde_installer.sh line 309:
    if which wget; then
       ^---^ SC2230: which is non-standard. Use builtin 'command -v' instead.


In mde_installer.sh line 311:
    elif which curl; then
         ^---^ SC2230: which is non-standard. Use builtin 'command -v' instead.


In mde_installer.sh line 353:
    if [ $(id -u) -ne 0 ]; then
         ^------^ SC2046: Quote this to prevent word splitting.


In mde_installer.sh line 362:
    local cores=$(nproc --all)
          ^---^ SC2155: Declare and assign separately to avoid masking return values.


In mde_installer.sh line 363:
    if [ $cores -lt $MIN_CORES ]; then
         ^----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
    if [ "$cores" -lt $MIN_CORES ]; then


In mde_installer.sh line 367:
    local mem_mb=$(free -m | grep Mem | awk '{print $2}')
          ^----^ SC2155: Declare and assign separately to avoid masking return values.


In mde_installer.sh line 368:
    if [ $mem_mb -lt $MIN_MEM_MB ]; then
         ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
    if [ "$mem_mb" -lt $MIN_MEM_MB ]; then


In mde_installer.sh line 372:
    local disk_space_mb=$(df -m . | tail -1 | awk '{print $4}')
          ^-----------^ SC2155: Declare and assign separately to avoid masking return values.


In mde_installer.sh line 373:
    if [ $disk_space_mb -lt $MIN_DISK_SPACE_MB ]; then
         ^------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
    if [ "$disk_space_mb" -lt $MIN_DISK_SPACE_MB ]; then


In mde_installer.sh line 386:
	lines=$(systemctl status $1 2>&1 | grep "Active: active" | wc -l)
                                 ^-- SC2086: Double quote to prevent globbing and word splitting.
                                           ^-------------------^ SC2126: Consider using grep -c instead of grep|wc -l.

Did you mean: 
	lines=$(systemctl status "$1" 2>&1 | grep "Active: active" | wc -l)


In mde_installer.sh line 388:
    if [ $lines -eq 0 ]; then
         ^----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
    if [ "$lines" -eq 0 ]; then


In mde_installer.sh line 400:
    local conflicting_apps=$(timeout 5m find /proc/*/fdinfo/ -type f -print0 2>/dev/null | xargs -r0 grep -Fl "fanotify mnt_id" 2>/dev/null | xargs -I {} -r sh -c 'cat "$(dirname {})/../cmdline"')
          ^--------------^ SC2155: Declare and assign separately to avoid masking return values.
                                                                                                                                                                   ^-- SC2016: Expressions don't expand in single quotes, use double quotes for that.


In mde_installer.sh line 402:
    if [ ! -z $conflicting_apps ]; then
         ^-- SC2236: Use -n instead of ! -z.
              ^---------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
    if [ ! -z "$conflicting_apps" ]; then


In mde_installer.sh line 417:
        set -- $t
               ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
        set -- "$t"


In mde_installer.sh line 419:
        if find_service $1; then
                        ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
        if find_service "$1"; then


In mde_installer.sh line 458:
        dpkg -s $1 2> /dev/null | grep Status | grep "install ok installed" 1> /dev/null
                ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
        dpkg -s "$1" 2> /dev/null | grep Status | grep "install ok installed" 1> /dev/null


In mde_installer.sh line 460:
        rpm --quiet --query $(get_rpm_proxy_params) $1
                            ^---------------------^ SC2046: Quote this to prevent word splitting.
                                                    ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
        rpm --quiet --query $(get_rpm_proxy_params) "$1"


In mde_installer.sh line 483:
    if [ ! -z "$pkgs_to_be_installed" ]; then
         ^-- SC2236: Use -n instead of ! -z.


In mde_installer.sh line 498:
        lines=$(ps axo pid,comm | grep "$PKG_MGR" | grep -v grep -c)
                ^-- SC2009: Consider using pgrep instead of grepping ps output.


In mde_installer.sh line 512:
    local packages=
          ^------^ SC2178: Variable was used as an array but is now assigned a string.


In mde_installer.sh line 514:
    local success=
          ^-----^ SC2034: success appears unused. Verify use (or export if used externally).


In mde_installer.sh line 524:
    install_required_pkgs ${packages[@]}
                          ^------------^ SC2068: Double quote array expansions to avoid re-splitting elements.


In mde_installer.sh line 553:
    local packages=
          ^------^ SC2178: Variable was used as an array but is now assigned a string.


In mde_installer.sh line 564:
    repo=packages-microsoft-com
         ^--------------------^ SC2100: Use $((..)) for arithmetics, e.g. i=$((i - 2))


In mde_installer.sh line 568:
        packages=($packages deltarpm)
                  ^-------^ SC2128: Expanding an array without an index only gives the first element.
                  ^-------^ SC2206: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.


In mde_installer.sh line 571:
    install_required_pkgs ${packages[@]}
                          ^------------^ SC2068: Double quote array expansions to avoid re-splitting elements.


In mde_installer.sh line 575:
        repo=packages-microsoft-com-prod
             ^-------------------------^ SC2100: Use $((..)) for arithmetics, e.g. i=$((i - 2))


In mde_installer.sh line 601:
    local packages=
          ^------^ SC2178: Variable was used as an array but is now assigned a string.


In mde_installer.sh line 611:
    repo=packages-microsoft-com
         ^--------------------^ SC2100: Use $((..)) for arithmetics, e.g. i=$((i - 2))


In mde_installer.sh line 614:
    install_required_pkgs ${packages[@]}
                          ^------------^ SC2068: Double quote array expansions to avoid re-splitting elements.


In mde_installer.sh line 620:
    lines=$($PKG_MGR_INVOKER lr | grep "packages-microsoft-com-$CHANNEL" | wc -l)
                                  ^-- SC2126: Consider using grep -c instead of grep|wc -l.


In mde_installer.sh line 622:
    if [ $lines -eq 0 ]; then
         ^----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
    if [ "$lines" -eq 0 ]; then


In mde_installer.sh line 648:
    local packages=
          ^------^ SC2178: Variable was used as an array but is now assigned a string.


In mde_installer.sh line 659:
    install_required_pkgs ${packages[@]}
                          ^------------^ SC2068: Double quote array expansions to avoid re-splitting elements.


In mde_installer.sh line 685:
            repo=packages-microsoft-com-prod
                 ^-------------------------^ SC2100: Use $((..)) for arithmetics, e.g. i=$((i - 2))


In mde_installer.sh line 731:
    for version in ${SUPPORTED_RHEL6_VERSIONS[@]}; do
                   ^----------------------------^ SC2068: Double quote array expansions to avoid re-splitting elements.


In mde_installer.sh line 744:
            if rhel6_supported_version $VERSION; then # support versions 6.7+
                                       ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
            if rhel6_supported_version "$VERSION"; then # support versions 6.7+


In mde_installer.sh line 792:
        PYTHON=$(which python || which python3)
                 ^---^ SC2230: which is non-standard. Use builtin 'command -v' instead.
                                 ^---^ SC2230: which is non-standard. Use builtin 'command -v' instead.


In mde_installer.sh line 794:
        if [ -z $PYTHON ]; then
                ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
        if [ -z "$PYTHON" ]; then


In mde_installer.sh line 824:
    if [ $(mdatp health --field passive_mode_enabled | tail -1) == "false" ]; then
         ^-- SC2046: Quote this to prevent word splitting.


In mde_installer.sh line 838:
        set -- $t
               ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
        set -- "$t"


In mde_installer.sh line 840:
            local set_tags=$(mdatp health --field edr_device_tags)
                  ^------^ SC2155: Declare and assign separately to avoid masking return values.


In mde_installer.sh line 843:
            local result=$(echo "$set_tags" | grep -q "\"key\":\"$1\""; echo "$?")
                  ^----^ SC2155: Declare and assign separately to avoid masking return values.


In mde_installer.sh line 844:
            if [ $result -eq 0 ]; then
                 ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
            if [ "$result" -eq 0 ]; then


In mde_installer.sh line 845:
                local value=$(echo "$set_tags" | grep -o "\"key\":\"$1\".*\"" | cut -d '"' -f 8)
                      ^---^ SC2155: Declare and assign separately to avoid masking return values.


In mde_installer.sh line 854:
                local tag_value="\"${@:2}\""
                                ^----------^ SC2124: Assigning an array to a string! Assign as array, or use * instead of @ to concatenate.


In mde_installer.sh line 1076:
if [ ! -z $PASSIVE_MODE ]; then
     ^-- SC2236: Use -n instead of ! -z.


In mde_installer.sh line 1080:
if [ ! -z $ONBOARDING_SCRIPT ]; then
     ^-- SC2236: Use -n instead of ! -z.
          ^----------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
if [ ! -z "$ONBOARDING_SCRIPT" ]; then

For more information:
  https://www.shellcheck.net/wiki/SC2068 -- Double quote array expansions to ...
  https://www.shellcheck.net/wiki/SC2070 -- -n doesn't work with unquoted arg...
  https://www.shellcheck.net/wiki/SC2034 -- success appears unused. Verify us...

I would also suggest addressing these warnings and errors.

@SvenMarquardt5772 SvenMarquardt5772 changed the title Use shellcheck for better compatibility Use shellcheck for better compatibility in mde_installer.sh Dec 7, 2022
@agarwalneetu
Copy link
Contributor

@SvenMarquardt5772, is this issue still open? can I start working on this?

@SvenMarquardt5772
Copy link
Author

@SvenMarquardt5772, is this issue still open? can I start working on this?

Yes, this is still an issue, This should be part of the pipeline.

@agarwalneetu
Copy link
Contributor

@SvenMarquardt5772, which pipeline you are referring to?

@SvenMarquardt5772
Copy link
Author

The one you are using for Continuous Integration. I suppose if you don't have one here on GitHub you have one internally where your tests are running after commits on main. Or whatever QA process you have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants