Add runcodeql.sh for "tox -e codeql" - Security check in python codes

[nhosoi]

I pulled out codeql command lines from the codeql-action output and put them into the runcodeql.sh script.

This is the sample output of tox -e codeql run against nbde_server.

[
  {
    "ruleId": "py/multiple-definition",
    "rule": {
      "id": "py/multiple-definition",
      "index": 143,
      "toolComponent": {
        "index": 25
      }
    },
    "message": {
      "text": "This assignment to 'rotate_result' is unnecessary as it is [redefined](1) before this value is used."
    },
    "locations": [
      {
        "physicalLocation": {
          "artifactLocation": {
            "uri": "library/nbde_server_tang.py",
            "uriBaseId": "%SRCROOT%",
            "index": 0
          },
          "region": {
            "startLine": 226,
            "startColumn": 5,
            "endColumn": 18
          }
        }
      }
    ],
    "partialFingerprints": {
      "primaryLocationLineHash": "cb955d5b01ee9e6e:1",
      "primaryLocationStartColumnFingerprint": "0"
    },
    "relatedLocations": [
      {
        "id": 1,
        "physicalLocation": {
          "artifactLocation": {
            "uri": "library/nbde_server_tang.py",
            "uriBaseId": "%SRCROOT%",
            "index": 0
          },
          "region": {
            "startLine": 258,
            "startColumn": 9,
            "endColumn": 22
          }
        },
        "message": {
          "text": "redefined"
        }
      }
    ]
  }
]
runcodeql.sh: Found 1 security issues.

To be honest, reading the code, I'm not convinced that "This assignment to 'rotate_result' is unnecessary"... Could someone understand this codeql result?
https://github.com/linux-system-roles/nbde_server/blob/main/library/nbde_server_tang.py#L258

[richm]

To me this is a false positive - it is saying that this assignment is unnecessary: https://github.com/linux-system-roles/nbde_server/blob/main/library/nbde_server_tang.py#L226 because rotate_result is set again before it is used at https://github.com/linux-system-roles/nbde_server/blob/main/library/nbde_server_tang.py#L264
But to me, that isn't true - if any line of code in https://github.com/linux-system-roles/nbde_server/blob/main/library/nbde_server_tang.py#L231-L256 raises an exception, and rotate_result wasn't set at 226, then it is possible line 264 will read rotate_result without it being set.

[richm]

because this is running in tox, you already have a "working" directory .tox/codeql and a "temp" directory .tox/codeql/.tmp
https://github.com/linux-system-roles/tox-lsr#environment-variables-available-for-test-scripts
The codeql command should be installed in $LSR_TOX_ENV_DIR/bin which is in the PATH when running in tox.

[richm]

You could probably make it idempotent:

if [ ! -f "$LSR_TOX_ENV_TMP_DIR/codeql-bundle-linux64.tar.gz" ]; then
  curl -o "$LSR_TOX_ENV_TMP_DIR/codeql-bundle-linux64.tar.gz"  "$CODEQLURL"
fi
if [ ! -f "$LSR_TOX_ENV_DIR/bin/codeql" ]; then
  tar xfz "$LSR_TOX_ENV_TMP_DIR/codeql-bundle-linux64.tar.gz" -C "$LSR_TOX_ENV_TMP_DIR"
  cp "$LSR_TOX_ENV_TMP_DIR/codeql" "$LSR_TOX_ENV_DIR/bin/codeql"
fi

[richm]

Suggested change

	(cd $GITHUBDIR; gh repo clone github/codeql-action)
	git clone https://github.com/github/codeql-action "$LSR_TOX_ENV_DIR/codeql-action"

[richm]

move this check to near the top so the script fails early if jq not found

[richm]

You can remove this check since you added it to the top

[nhosoi]

To me this is a false positive - it is saying that this assignment is unnecessary: https://github.com/linux-system-roles/nbde_server/blob/main/library/nbde_server_tang.py#L226 because rotate_result is set again before it is used at https://github.com/linux-system-roles/nbde_server/blob/main/library/nbde_server_tang.py#L264 But to me, that isn't true - if any line of code in https://github.com/linux-system-roles/nbde_server/blob/main/library/nbde_server_tang.py#L231-L256 raises an exception, and rotate_result wasn't set at 226, then it is possible line 264 will read rotate_result without it being set.

Ok. Thanks. Then, do you think we'd better find a way to skip testing on the line as noqa or wokeignore (if any though...)???

[richm]

To me this is a false positive - it is saying that this assignment is unnecessary: https://github.com/linux-system-roles/nbde_server/blob/main/library/nbde_server_tang.py#L226 because rotate_result is set again before it is used at https://github.com/linux-system-roles/nbde_server/blob/main/library/nbde_server_tang.py#L264 But to me, that isn't true - if any line of code in https://github.com/linux-system-roles/nbde_server/blob/main/library/nbde_server_tang.py#L231-L256 raises an exception, and rotate_result wasn't set at 226, then it is possible line 264 will read rotate_result without it being set.

Ok. Thanks. Then, do you think we'd better find a way to skip testing on the line as noqa or wokeignore (if any though...)???

Without using the UI in github, you can only exclude entire files. There isn't a comment mechanism - github/codeql#11427
So I guess you'll have to go through the issues in the github UI and handle the alerts there.

[richm]

Suggested change

	[testenv:codql]
	[testenv:codeql]

[nhosoi]

shame shame Thanks, @richm!