Skip to content

tests: resolve CodeQL Python alerts in the CLI test helpers - #2499

Merged
ronaldtse merged 2 commits into
mainfrom
fix-codeql-python-alerts
Sep 25, 2026
Merged

ronaldtse merged 2 commits into
mainfrom
fix-codeql-python-alerts

Conversation

@ronaldtse

Copy link
Copy Markdown
Contributor

This pull request resolves the four open CodeQL Python alerts in the CLI test helpers, which have been failing the CodeQL check on pull requests that include the files through their merge base, for example #2494.

The high-severity alert (#454, clear-text logging of sensitive information) points at the command line that run_proc() logs. The password values that flow into it are the fake test passwords of the harness, and they are masked by _redact_sensitive_args() before the line is written, so the warning is a false positive for this location. The alert is therefore suppressed with an inline codeql[py/clear-text-logging-sensitive-data] comment, which keeps the reasoning next to the line it covers and keeps the suppression visible in the code scanning audit view rather than hiding it in a configuration file.

The remaining three alerts are resolved in the code. The pass_fl and pass_cp locals of run_proc_windows() (#626 and #627) are initialised to None, which makes the pairing of the setup and cleanup branches explicit. The gpg transient-failure retry wrapper (#630) ends with an explicit raise after its retry loop, so the function no longer mixes an implicit fall-through return with explicit returns; the branch is unreachable because the last attempt always re-raises.

I verified the changes locally with the CodeQL CLI against a security-and-quality run, matching the query suite configured in the workflow: the uninitialized-local-variable and mixed-returns alerts no longer appear, and the only remaining result is the suppressed line. I would also like to thank @ni4 and @antonsviridenko in advance for taking a look whenever their time allows.

The high-severity clear-text logging alert on run_proc() is addressed
with an inline suppression: the password values handled by the test
harness are fake, and they are masked by _redact_sensitive_args()
before the command line is logged, so the warning is a false positive
for this line. The potentially uninitialized pass_fl and pass_cp
locals of run_proc_windows() are initialised to None, and the gpg
transient-failure retry wrapper ends with an explicit raise so the
function no longer mixes an implicit fall-through return with explicit
returns.
Comment thread src/tests/cli_common.py Fixed
@codecov

codecov Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.45%. Comparing base (26482f6) to head (5adac3c).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2499   +/-   ##
=======================================
  Coverage   85.45%   85.45%           
=======================================
  Files         125      125           
  Lines       23042    23042           
=======================================
  Hits        19691    19691           
  Misses       3351     3351           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Inline suppression comments apply to the statement on the line that
follows the comment, so the annotation must sit on its own line
directly above the logging call rather than at the end of it.
@ronaldtse

Copy link
Copy Markdown
Contributor Author

The CodeQL check on this pull request is now green, which confirms the resolution of all four open Python alerts.

For the high-severity clear-text logging alert, the suppression comment initially sat at the end of the logging line and had no effect, because an inline codeql[...] annotation applies to the statement on the line that follows it. Moving the annotation onto its own line directly above the call resolved the alert, and the repeated failure of the check turned into a pass once the placement was corrected. The two uninitialized-local-variable alerts and the mixed-returns alert are resolved in code by initialising pass_fl and pass_cp and by ending the retry wrapper with an explicit raise.

I verified the changes locally with the CodeQL CLI 2.27.0 and the same security-and-quality query suite that the workflow uses, and both analyses agree. Since this change touches test-support code, reviews from @ni4 and @antonsviridenko are kindly requested whenever your time allows. Thank you.

@ni4 ni4 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ronaldtse
ronaldtse merged commit bdabdf9 into main Sep 25, 2026
144 of 146 checks passed
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

Successfully merging this pull request may close these issues.

3 participants