From d42b2bc00139df51a62469dba5e81774378939a9 Mon Sep 17 00:00:00 2001 From: Ben Date: Tue, 27 Oct 2026 02:25:08 -0700 Subject: [PATCH 1/3] fix(cxx): require identifier boundaries for unsafe API names --- .../languages/cxx/detectors/security.py | 2 +- .../languages/cxx/tests/test_security.py | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/desloppify/languages/cxx/detectors/security.py b/desloppify/languages/cxx/detectors/security.py index 2d4570856..c63219880 100644 --- a/desloppify/languages/cxx/detectors/security.py +++ b/desloppify/languages/cxx/detectors/security.py @@ -248,7 +248,7 @@ def _normalize_kind(check_id: str, message: str) -> str | None: ): return "command_injection" - if any(api in text for api in _UNSAFE_C_STRING_APIS): + if re.search(r"\b(?:" + "|".join(_UNSAFE_C_STRING_APIS) + r")\b", text): return "unsafe_c_string" if "unsafebufferhandling" in check: return "unsafe_c_string" diff --git a/desloppify/languages/cxx/tests/test_security.py b/desloppify/languages/cxx/tests/test_security.py index d18aa4082..7eaa868e6 100644 --- a/desloppify/languages/cxx/tests/test_security.py +++ b/desloppify/languages/cxx/tests/test_security.py @@ -611,3 +611,22 @@ def _fake_run_tool_result(cmd, path, parser, **_kwargs): assert len(result.entries) == 1 assert result.entries[0]["file"] == str(source.resolve()) + + +def test_missing_widget_include_is_not_an_unsafe_gets_call(tmp_path): + entry = { + "file": str(tmp_path / "controller.cpp"), + "line": 11, + "severity": "information", + "check_id": "missingInclude", + "message": 'Include file: "Widgets/Input/SVirtualJoystick.h" not found.', + "source": "cppcheck", + } + assert security_mod._normalize_tool_entry(entry) is None + + +def test_unsafe_string_api_names_require_identifier_boundaries(): + for api in security_mod._UNSAFE_C_STRING_APIS: + assert security_mod._normalize_kind("", f"Calling '{api}()' is unsafe") == "unsafe_c_string" + assert security_mod._normalize_kind(f"clang-analyzer-security.insecureAPI.{api}", "") == "unsafe_c_string" + assert security_mod._normalize_kind("", f"Include file: my_{api}_wrapper.h not found") is None From 10ba1cf859b80dac825cbe9c79e075d5d730dd05 Mon Sep 17 00:00:00 2001 From: Ben Date: Tue, 27 Oct 2026 03:29:55 -0700 Subject: [PATCH 2/3] fix(cxx): report cppcheck configuration failures as reduced coverage --- .../languages/cxx/cppcheck_diagnostics.py | 35 ++++++++++++++ .../languages/cxx/detectors/security.py | 3 ++ desloppify/languages/cxx/phases.py | 11 +++-- .../cxx/tests/test_cppcheck_phase.py | 47 +++++++++++++++++++ .../languages/cxx/tests/test_security.py | 13 +++++ 5 files changed, 105 insertions(+), 4 deletions(-) create mode 100644 desloppify/languages/cxx/cppcheck_diagnostics.py diff --git a/desloppify/languages/cxx/cppcheck_diagnostics.py b/desloppify/languages/cxx/cppcheck_diagnostics.py new file mode 100644 index 000000000..a34b62599 --- /dev/null +++ b/desloppify/languages/cxx/cppcheck_diagnostics.py @@ -0,0 +1,35 @@ +"""Distinguish cppcheck setup/parse failures from reliable code diagnostics.""" + +from __future__ import annotations + +from dataclasses import replace +from pathlib import Path + +from desloppify.languages._framework.generic_parts.tool_runner import ToolRunResult + +_CONFIGURATION_IDS = frozenset({"missingInclude", "missingIncludeSystem", "unknownMacro"}) + + +def configuration_checked_result(result: ToolRunResult) -> ToolRunResult: + """Fail the invocation when cppcheck could not understand its translation units. + + Cppcheck can exit zero while reporting missing headers or unsupported macros. + The existing batch runners retry individual files and preserve regex security + fallback; returning an error here activates that coverage accounting. + """ + if result.status == "error": + return result + for entry in result.entries: + message = str(entry.get("message", "")) + is_missing_include = "Include file:" in message and "not found" in message + is_unknown_macro = "unknown macro here" in message and "configure" in message + is_cpp_header_as_c = ( + "invalid C code" in message + and Path(str(entry.get("file", ""))).suffix.lower() in {".h", ".hh", ".hpp", ".hxx"} + ) + if entry.get("check_id") in _CONFIGURATION_IDS or is_missing_include or is_unknown_macro or is_cpp_header_as_c: + return replace( + result, entries=[], status="error", error_kind="tool_configuration", + message=f"cppcheck requires include paths, macro definitions or language configuration: {message}", + ) + return result diff --git a/desloppify/languages/cxx/detectors/security.py b/desloppify/languages/cxx/detectors/security.py index c63219880..e84ef66f0 100644 --- a/desloppify/languages/cxx/detectors/security.py +++ b/desloppify/languages/cxx/detectors/security.py @@ -23,6 +23,8 @@ run_tool_result, ) +from desloppify.languages.cxx.cppcheck_diagnostics import configuration_checked_result + logger = logging.getLogger(__name__) _SOURCE_SUFFIXES = frozenset({".c", ".cc", ".cpp", ".cxx"}) @@ -495,6 +497,7 @@ def _run_cppcheck_invocation(scan_root: Path, files: list[str]) -> CxxToolScanRe scan_root, _parse_cppcheck_output, ) + result = configuration_checked_result(result) state = _tool_result_state(result) return CxxToolScanResult( tool="cppcheck", diff --git a/desloppify/languages/cxx/phases.py b/desloppify/languages/cxx/phases.py index 42d95c7fa..d1e82735a 100644 --- a/desloppify/languages/cxx/phases.py +++ b/desloppify/languages/cxx/phases.py @@ -17,6 +17,7 @@ from desloppify.languages._framework.generic_parts.parsers import PARSERS from desloppify.languages._framework.generic_parts.tool_factories import _record_tool_failure_coverage from desloppify.languages._framework.generic_parts.tool_runner import run_tool_result +from desloppify.languages.cxx.cppcheck_diagnostics import configuration_checked_result from desloppify.languages.cxx._helpers import build_cxx_dep_graph from desloppify.languages.cxx.extractors import find_cxx_files from desloppify.state_io import Issue @@ -44,10 +45,12 @@ def _cppcheck_file_args(files: list[str]) -> str: def _run_cppcheck_batch(scan_root: Path, files: list[str]): - return run_tool_result( - f"{_CPPCHECK_CMD_PREFIX} {_cppcheck_file_args(files)}", - scan_root, - PARSERS["gnu"], + return configuration_checked_result( + run_tool_result( + f"{_CPPCHECK_CMD_PREFIX} {_cppcheck_file_args(files)}", + scan_root, + PARSERS["gnu"], + ) ) diff --git a/desloppify/languages/cxx/tests/test_cppcheck_phase.py b/desloppify/languages/cxx/tests/test_cppcheck_phase.py index 2d4c6ff54..3c8b0ef84 100644 --- a/desloppify/languages/cxx/tests/test_cppcheck_phase.py +++ b/desloppify/languages/cxx/tests/test_cppcheck_phase.py @@ -146,3 +146,50 @@ def _fake_run_tool_result(_cmd, _path, _parser, **_kwargs): assert len(issues) == 2 assert issues[0]["id"] != issues[1]["id"] assert signals == {"cppcheck_issue": 2} + + +def test_cppcheck_configuration_failure_retries_files_and_preserves_reliable_findings(tmp_path, monkeypatch): + monkeypatch.setattr(cxx_phases, "find_cxx_files", lambda _: ["broken.cpp", "clean.cpp"]) + calls = [] + + def fake_run(cmd, _path, _parser): + calls.append(cmd) + if "broken.cpp" in cmd: + return ToolRunResult(entries=[{ + "file": "broken.cpp", "line": 5, + "message": "There is an unknown macro here somewhere. Configuration is required. If GENERATED_BODY is a macro then please configure it.", + }], status="ok", returncode=0) + return ToolRunResult(entries=[{"file": "clean.cpp", "line": 7, "message": "Variable x is unused"}], status="ok", returncode=0) + + monkeypatch.setattr(cxx_phases, "run_tool_result", fake_run) + lang = SimpleNamespace(detector_coverage={}, coverage_warnings=[]) + issues, signals = cxx_phases.phase_cppcheck_issue(tmp_path, lang) + assert len(calls) == 3 + assert [i["summary"] for i in issues] == ["Variable x is unused"] + assert signals == {"cppcheck_issue": 1} + assert lang.detector_coverage["cppcheck_issue"]["reason"] == "tool_configuration" + + +def test_cppcheck_missing_include_is_coverage_failure_not_code_issue(tmp_path, monkeypatch): + monkeypatch.setattr(cxx_phases, "find_cxx_files", lambda _: ["controller.cpp"]) + monkeypatch.setattr(cxx_phases, "run_tool_result", lambda *_: ToolRunResult(entries=[{ + "file": "controller.cpp", "line": 11, + "message": 'information: Include file: "Widgets/Input/SVirtualJoystick.h" not found.', + }], status="ok", returncode=0)) + lang = SimpleNamespace(detector_coverage={}, coverage_warnings=[]) + issues, signals = cxx_phases.phase_cppcheck_issue(tmp_path, lang) + assert issues == [] + assert signals == {} + assert lang.detector_coverage["cppcheck_issue"]["status"] == "reduced" + + +def test_cppcheck_cpp_header_parsed_as_c_is_configuration_failure_only_for_headers(tmp_path, monkeypatch): + for filename, reduced in [("widget.hpp", True), ("widget.c", False)]: + monkeypatch.setattr(cxx_phases, "find_cxx_files", lambda _, file=filename: [file]) + monkeypatch.setattr(cxx_phases, "run_tool_result", lambda *_, file=filename: ToolRunResult(entries=[{ + "file": file, "line": 4, "message": "error: Code 'template<...' is invalid C code.", + }], status="ok", returncode=0)) + lang = SimpleNamespace(detector_coverage={}, coverage_warnings=[]) + issues, _ = cxx_phases.phase_cppcheck_issue(tmp_path, lang) + assert bool(lang.detector_coverage) is reduced + assert len(issues) == (0 if reduced else 1) diff --git a/desloppify/languages/cxx/tests/test_security.py b/desloppify/languages/cxx/tests/test_security.py index 7eaa868e6..a1df6ce36 100644 --- a/desloppify/languages/cxx/tests/test_security.py +++ b/desloppify/languages/cxx/tests/test_security.py @@ -630,3 +630,16 @@ def test_unsafe_string_api_names_require_identifier_boundaries(): assert security_mod._normalize_kind("", f"Calling '{api}()' is unsafe") == "unsafe_c_string" assert security_mod._normalize_kind(f"clang-analyzer-security.insecureAPI.{api}", "") == "unsafe_c_string" assert security_mod._normalize_kind("", f"Include file: my_{api}_wrapper.h not found") is None + + +def test_cppcheck_configuration_failure_keeps_regex_security_fallback(tmp_path, monkeypatch): + source = tmp_path / "unsafe.cpp" + source.write_text("void copy(char* a, const char* b) { strcpy(a,b); }\n") + monkeypatch.setattr(security_mod.shutil, "which", lambda name: "/usr/bin/cppcheck" if name == "cppcheck" else None) + monkeypatch.setattr(security_mod, "run_tool_result", lambda *_: ToolRunResult(entries=[{ + "file": str(source), "line": 1, "severity": "error", "check_id": "unknownMacro", + "message": "Unknown macro requires configuration", "source": "cppcheck", + }], status="ok", returncode=0)) + result = security_mod.detect_cxx_security([str(source)], None) + assert result.coverage.status == "reduced" + assert any(e["detail"]["kind"] == "unsafe_c_string" and e["detail"]["source"] == "regex" for e in result.entries) From a495956a7b506589c7ec0a01b3e1c6e32ec87c7d Mon Sep 17 00:00:00 2001 From: Ben Date: Tue, 27 Oct 2026 04:02:08 -0700 Subject: [PATCH 3/3] Keep phase-level configuration handling in existing upstream PR 864 --- .../languages/cxx/cppcheck_diagnostics.py | 2 +- desloppify/languages/cxx/phases.py | 11 ++--- .../cxx/tests/test_cppcheck_phase.py | 47 ------------------- 3 files changed, 5 insertions(+), 55 deletions(-) diff --git a/desloppify/languages/cxx/cppcheck_diagnostics.py b/desloppify/languages/cxx/cppcheck_diagnostics.py index a34b62599..89a4899db 100644 --- a/desloppify/languages/cxx/cppcheck_diagnostics.py +++ b/desloppify/languages/cxx/cppcheck_diagnostics.py @@ -14,7 +14,7 @@ def configuration_checked_result(result: ToolRunResult) -> ToolRunResult: """Fail the invocation when cppcheck could not understand its translation units. Cppcheck can exit zero while reporting missing headers or unsupported macros. - The existing batch runners retry individual files and preserve regex security + The security runner retries individual files and preserves regex security fallback; returning an error here activates that coverage accounting. """ if result.status == "error": diff --git a/desloppify/languages/cxx/phases.py b/desloppify/languages/cxx/phases.py index d1e82735a..42d95c7fa 100644 --- a/desloppify/languages/cxx/phases.py +++ b/desloppify/languages/cxx/phases.py @@ -17,7 +17,6 @@ from desloppify.languages._framework.generic_parts.parsers import PARSERS from desloppify.languages._framework.generic_parts.tool_factories import _record_tool_failure_coverage from desloppify.languages._framework.generic_parts.tool_runner import run_tool_result -from desloppify.languages.cxx.cppcheck_diagnostics import configuration_checked_result from desloppify.languages.cxx._helpers import build_cxx_dep_graph from desloppify.languages.cxx.extractors import find_cxx_files from desloppify.state_io import Issue @@ -45,12 +44,10 @@ def _cppcheck_file_args(files: list[str]) -> str: def _run_cppcheck_batch(scan_root: Path, files: list[str]): - return configuration_checked_result( - run_tool_result( - f"{_CPPCHECK_CMD_PREFIX} {_cppcheck_file_args(files)}", - scan_root, - PARSERS["gnu"], - ) + return run_tool_result( + f"{_CPPCHECK_CMD_PREFIX} {_cppcheck_file_args(files)}", + scan_root, + PARSERS["gnu"], ) diff --git a/desloppify/languages/cxx/tests/test_cppcheck_phase.py b/desloppify/languages/cxx/tests/test_cppcheck_phase.py index 3c8b0ef84..2d4c6ff54 100644 --- a/desloppify/languages/cxx/tests/test_cppcheck_phase.py +++ b/desloppify/languages/cxx/tests/test_cppcheck_phase.py @@ -146,50 +146,3 @@ def _fake_run_tool_result(_cmd, _path, _parser, **_kwargs): assert len(issues) == 2 assert issues[0]["id"] != issues[1]["id"] assert signals == {"cppcheck_issue": 2} - - -def test_cppcheck_configuration_failure_retries_files_and_preserves_reliable_findings(tmp_path, monkeypatch): - monkeypatch.setattr(cxx_phases, "find_cxx_files", lambda _: ["broken.cpp", "clean.cpp"]) - calls = [] - - def fake_run(cmd, _path, _parser): - calls.append(cmd) - if "broken.cpp" in cmd: - return ToolRunResult(entries=[{ - "file": "broken.cpp", "line": 5, - "message": "There is an unknown macro here somewhere. Configuration is required. If GENERATED_BODY is a macro then please configure it.", - }], status="ok", returncode=0) - return ToolRunResult(entries=[{"file": "clean.cpp", "line": 7, "message": "Variable x is unused"}], status="ok", returncode=0) - - monkeypatch.setattr(cxx_phases, "run_tool_result", fake_run) - lang = SimpleNamespace(detector_coverage={}, coverage_warnings=[]) - issues, signals = cxx_phases.phase_cppcheck_issue(tmp_path, lang) - assert len(calls) == 3 - assert [i["summary"] for i in issues] == ["Variable x is unused"] - assert signals == {"cppcheck_issue": 1} - assert lang.detector_coverage["cppcheck_issue"]["reason"] == "tool_configuration" - - -def test_cppcheck_missing_include_is_coverage_failure_not_code_issue(tmp_path, monkeypatch): - monkeypatch.setattr(cxx_phases, "find_cxx_files", lambda _: ["controller.cpp"]) - monkeypatch.setattr(cxx_phases, "run_tool_result", lambda *_: ToolRunResult(entries=[{ - "file": "controller.cpp", "line": 11, - "message": 'information: Include file: "Widgets/Input/SVirtualJoystick.h" not found.', - }], status="ok", returncode=0)) - lang = SimpleNamespace(detector_coverage={}, coverage_warnings=[]) - issues, signals = cxx_phases.phase_cppcheck_issue(tmp_path, lang) - assert issues == [] - assert signals == {} - assert lang.detector_coverage["cppcheck_issue"]["status"] == "reduced" - - -def test_cppcheck_cpp_header_parsed_as_c_is_configuration_failure_only_for_headers(tmp_path, monkeypatch): - for filename, reduced in [("widget.hpp", True), ("widget.c", False)]: - monkeypatch.setattr(cxx_phases, "find_cxx_files", lambda _, file=filename: [file]) - monkeypatch.setattr(cxx_phases, "run_tool_result", lambda *_, file=filename: ToolRunResult(entries=[{ - "file": file, "line": 4, "message": "error: Code 'template<...' is invalid C code.", - }], status="ok", returncode=0)) - lang = SimpleNamespace(detector_coverage={}, coverage_warnings=[]) - issues, _ = cxx_phases.phase_cppcheck_issue(tmp_path, lang) - assert bool(lang.detector_coverage) is reduced - assert len(issues) == (0 if reduced else 1)