Repository navigation
ci(codeql): stop clear-text-logging false positives coming back each time a log line moves #2130
Description
CodeQL keeps re-raising the same py/clear-text-logging-sensitive-data false positive. Each time a PR moves one of the logger.debug lines in pcapkit/foundation/extraction.py, the alert comes back under a new number, so every dismissal is undone.
- New distribution [0.14.0] #25, New distribution [0.14.2] #28: the
reading from the pre-opened stream %rline (fin), dismissed twice. - New distribution [0.14.1] #26, now InfoClass instanciation calls
os.stat, ruining performance #29: thecleaning up after %d frame(s) from %sline in_cleanup(self._ifnm). New distribution [0.14.1] #26 was dismissed as a false positive. fix(foundation): close the engine too when the Extractor constructor raises after choosing one (#2107) #2119 moved it, and InfoClass instanciation callsos.stat, ruining performance #29 is open atextraction.py:1841. - Remove .vscode once for good #27: the same rule in
pcapkit/utilities/warnings.py.
CodeQL's message is "logs sensitive data (secret)". No secret is involved: these are a capture's file name and stream.
Proposed: find which name or flow CodeQL's sensitive-data heuristic is matching, then fix it once rather than per alert. In order of preference:
- If the source is a misleading identifier, rename it.
- If not, add a CodeQL config whose
query-filtersexclude this one rule. Thecodeql-analysis.ymlinit step would point at that config.
In the meantime, #29 can be dismissed as a false positive, as #26 was.
Activity
JarryShaw commentedon Oct 24, 2026
Root cause, from main's SARIF (CodeQL 2.27.3): CodeQL flags data as "secret" by its name alone. The test is the regex
(?is).*((?<!is|is_)secret|…trusted…|confidential).*inSensitiveDataHeuristics.qll.extraction.pyalert (Fixed minor bugs #20–Add .dockerignore #24, New distribution [0.14.2] #28, InfoClass instanciation callsos.stat, ruining performance #29) starts intests/. The sources are thesecretsparameter or variable of the pcapng DSB/key-log tests:test_pcapng_dsb_secrets_overrun_1980_unit.py,test_pcapng_tls_keylog_malformed_1985_unit.py.extract(fin=…)→fin→self._ifnm→logger.debug. Fixed minor bugs #19 (py/clear-text-storage-sensitive-data,corekit/io.py) has the same sources.self.secrets_length, a length, logged throughwarn().secrets_*are public schema fields.Which fix? I recommend option 1.
paths-ignore: [tests]in a CodeQL config. This removes every test-sourced alert (Fixed minor bugs #19– New distribution [0.14.1] #26, New distribution [0.14.2] #28, InfoClass instanciation callsos.stat, ruining performance #29) and keeps both rules active for library code, which is where a real leak would matter. Remove .vscode once for good #27 needs one more dismissal; it only returns if that line moves.query-filtersexclusions forpy/clear-text-logging-sensitive-data(and-storage-). This covers Remove .vscode once for good #27 too, but turns those rules off for the whole repo.A prototype of option 2, with a test, is ready. Switching it to option 1 is small.
Separately: the #1985 malformed-key-log warnings put the key-log line itself into the warning text. That is the one place real key material could reach a log, and CodeQL does not flag it. Should those warnings print only the line's position or length?
JarryShaw commentedon Oct 24, 2026
your recommendation?
JarryShaw commentedon Oct 24, 2026
Recommendations:
paths-ignore: [tests]in.github/codeql/codeql-config.yml, referenced from theInitialize CodeQLstep.secretsnames. Not scanningtests/removes them all (Fixed minor bugs #19– New distribution [0.14.1] #26, New distribution [0.14.2] #28, InfoClass instanciation callsos.stat, ruining performance #29) at the root.pcapkit/, where a real leak would matter.secrets_lengthinwarnings.py) gets one manual dismissal. It only comes back if that line moves.If you agree, I'll switch the prototype to option 1, file the #1985 change, and open both PRs.
JarryShaw commentedon Oct 24, 2026
sounds good. recommendation taken.
JarryShaw commentedon Oct 24, 2026
Thanks. Going ahead with both:
.github/codeql/codeql-config.ymlwithpaths-ignore: [tests], referenced from theInitialize CodeQLstep, plus a pinning test. It goes up as a PR under this issue.#27 (
secrets_lengthinwarnings.py) still needs one manual dismissal after the config lands.JarryShaw commentedon Oct 24, 2026
Correction to point 2 above: the #1985 warnings already do not quote the key-log line. I relayed an agent's claim without checking it. On
main,_malformed_line_messageinpcapkit/protocols/schema/misc/pcapng.pybuilds both the TLS and the WireGuard warning.So there is nothing to change there, and I'm not filing that follow-up. Only the CodeQL config (option 1) goes ahead.
ci(codeql): stop scanning tests/, the source of the recurring clear-t…