From 0b41a6b01e112b447ef9e0a77c5dbc5ae4da7fbf Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sat, 24 Oct 2026 00:42:08 -0400 Subject: [PATCH] ci(codeql): stop scanning tests/, the source of the recurring clear-text-logging false positives (#2130) py/clear-text-logging-sensitive-data and py/clear-text-storage-sensitive-data flagged pcapkit/ lines whose data flow began at a test fixture's `secrets` name, and each dismissal was lost when a pull request moved the line. - .github/codeql/codeql-config.yml: new; paths-ignore: [tests], nothing else. - codeql-analysis.yml: Initialize CodeQL reads it through config-file. - tests/project/test_codeql_config_2130.py: pins the input and the config. - docs/source/contributing/workflows.rst: CodeQL Configuration section. --- .github/codeql/codeql-config.yml | 15 +++++ .github/workflows/codeql-analysis.yml | 3 + docs/source/contributing/workflows.rst | 28 +++++++++ tests/project/test_codeql_config_2130.py | 76 ++++++++++++++++++++++++ 4 files changed, 122 insertions(+) create mode 100644 .github/codeql/codeql-config.yml create mode 100644 tests/project/test_codeql_config_2130.py diff --git a/.github/codeql/codeql-config.yml b/.github/codeql/codeql-config.yml new file mode 100644 index 0000000000..34f11922a8 --- /dev/null +++ b/.github/codeql/codeql-config.yml @@ -0,0 +1,15 @@ +# Read by `Initialize CodeQL` in codeql-analysis.yml, through its `config-file` +# input. +name: "PyPCAPKit CodeQL config" + +# #2130: the py/clear-text-logging-sensitive-data and +# py/clear-text-storage-sensitive-data alerts that came back each time a pull +# request moved a line in pcapkit/ began in tests/. A test that builds a pcapng +# Decryption Secrets Block calls its bytes `secrets`, and CodeQL takes a value +# whose name says secret for secret data. Python is not built, so an ignored +# path is left out of the database and cannot be a flow source. Both queries, +# and every other, still run over pcapkit/. The alert at the logger call in +# pcapkit/utilities/warnings.py begins there instead, at the Decryption Secrets +# Block's own `secrets_length` field, so this does not clear it. +paths-ignore: + - tests diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 94f8b974fa..1cd704e2c5 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -38,8 +38,11 @@ jobs: - name: Checkout repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + # #2130: the config leaves tests/ out of the scan. - name: Initialize CodeQL uses: github/codeql-action/init@6f2ac81f5cd81fe351a197ba0b00d91d03cd2aee # v4.39.0 + with: + config-file: ./.github/codeql/codeql-config.yml - name: Autobuild uses: github/codeql-action/autobuild@6f2ac81f5cd81fe351a197ba0b00d91d03cd2aee # v4.39.0 diff --git a/docs/source/contributing/workflows.rst b/docs/source/contributing/workflows.rst index 15066257ac..e3bc3a7693 100644 --- a/docs/source/contributing/workflows.rst +++ b/docs/source/contributing/workflows.rst @@ -1624,6 +1624,34 @@ result of ``analyze`` stands. Where the alerts cannot be read, the step writes a warning, the report marks them unavailable, and ``analyze``'s result stands unchanged. +.. _CodeQL Configuration: + +CodeQL Configuration (`#2130 `__) +------------------------------------------------------------------------------------- + +``Initialize CodeQL`` in ``codeql-analysis.yml`` reads +:file:`.github/codeql/codeql-config.yml` through its ``config-file`` input. The +config's ``paths-ignore`` leaves out :file:`tests` and changes nothing else, so the +default queries still run over :mod:`pcapkit` and every other path. Python is not +built, so an ignored file is left out of the analysis (`Specifying directories to +scan +`__) +and cannot be the source of a data flow. + +``py/clear-text-logging-sensitive-data`` and +``py/clear-text-storage-sensitive-data`` call a value secret from its name alone, +by CodeQL's `sensitive-data heuristics +`__. +The tests that build a pcapng Decryption Secrets Block call its bytes +``secrets``. Through :func:`~pcapkit.interface.core.extract` those bytes reached +``logger`` calls in :mod:`pcapkit.foundation.extraction` and the buffer file's +write in :mod:`pcapkit.corekit.io`, and each alert's dismissal was lost whenever +a pull request moved the line it named. The alert on the ``logger`` call in +:mod:`pcapkit.utilities.warnings` begins in :mod:`pcapkit` itself, at the block's +``secrets_length`` field, the spec's name for it, so it is still raised and still +dismissed by hand. :file:`tests/project/test_codeql_config_2130.py` pins the +config and the input. + .. _Action Pins: Action Pins (`#1968 `__) diff --git a/tests/project/test_codeql_config_2130.py b/tests/project/test_codeql_config_2130.py new file mode 100644 index 0000000000..3db6529dfc --- /dev/null +++ b/tests/project/test_codeql_config_2130.py @@ -0,0 +1,76 @@ +# -*- coding: utf-8 -*- +"""#2130: CodeQL scans everything but :file:`tests`. + +``py/clear-text-logging-sensitive-data`` and +``py/clear-text-storage-sensitive-data`` call a value secret from its name +alone. The tests that build a pcapng Decryption Secrets Block call its bytes +``secrets``, and those bytes reached ``logger`` calls in :mod:`pcapkit`, as a +capture's stream, its name, or its magic number, and a buffer file's write. +Each dismissal was lost when a +pull request moved the line the alert named. Python is not built, so a path the +config ignores is not extracted and cannot be a flow source. + +* :class:`TestTheInitStep` checks ``Initialize CodeQL`` in + :file:`codeql-analysis.yml` reads :file:`.github/codeql/codeql-config.yml` + and nothing else. +* :class:`TestTheConfig` checks the config ignores :file:`tests` and changes + nothing else: every query still runs, over every other path. + +""" + +from __future__ import annotations + +import unittest +from typing import TYPE_CHECKING + +from tests._tiers import ROOT +from tests._workflows import load, loads + +if TYPE_CHECKING: + from typing import Any + +#: Test tier (#1958): reads a workflow and the CodeQL config, a question about the tree. +__tier__ = 'LATEST' + +NAME = 'codeql-analysis.yml' +CONFIG = '.github/codeql/codeql-config.yml' + + +def config() -> 'dict[str, Any]': + return loads((ROOT / CONFIG).read_text(encoding='utf-8')) + + +class TestTheInitStep(unittest.TestCase): + """``Initialize CodeQL`` names the config file.""" + + def test_it_reads_the_config(self) -> None: + [step] = [entry for entry in load(NAME)['jobs']['analyze']['steps'] + if entry.get('name') == 'Initialize CodeQL'] + self.assertEqual(step.get('with'), {'config-file': f'./{CONFIG}'}) + self.assertTrue((ROOT / CONFIG).is_file()) + + def test_no_other_step_passes_a_config(self) -> None: + """An inline ``config`` would take precedence over the file.""" + for entry in load(NAME)['jobs']['analyze']['steps']: + with self.subTest(step=entry.get('name')): + self.assertNotIn('config', entry.get('with') or {}) + if entry.get('name') != 'Initialize CodeQL': + self.assertNotIn('config-file', entry.get('with') or {}) + + +class TestTheConfig(unittest.TestCase): + """:file:`tests` left out, and the scan otherwise as it was.""" + + def test_only_the_tests_are_ignored(self) -> None: + self.assertEqual(config()['paths-ignore'], ['tests']) + + def test_every_query_still_runs(self) -> None: + spec = config() + for key in ('query-filters', 'paths', 'disable-default-queries', 'queries', 'packs'): + with self.subTest(key=key): + self.assertNotIn(key, spec) + self.assertEqual(sorted(spec), ['name', 'paths-ignore']) + + +if __name__ == '__main__': + unittest.main()