diff --git a/AUTHORS b/AUTHORS index d77fa9dca46..62fd771b0d9 100644 --- a/AUTHORS +++ b/AUTHORS @@ -405,6 +405,7 @@ Tomo Dote Toralf Förster Troshin V.S. Tyson Nottingham +Usman Majid Valentin Batz Valerii Lashmanov Vasily Maslyukov diff --git a/Makefile b/Makefile index d71c614e903..cb2c2234bc7 100644 --- a/Makefile +++ b/Makefile @@ -171,7 +171,7 @@ ifndef INCLUDE_FOR_CLI endif ifndef INCLUDE_FOR_TEST - INCLUDE_FOR_TEST=-Ilib -Ifrontend -Icli -isystem externals/simplecpp -isystem externals/tinyxml2 + INCLUDE_FOR_TEST=-Ilib -Ifrontend -Icli -isystem externals/simplecpp -isystem externals/tinyxml2 -isystem externals/picojson endif BIN=$(DESTDIR)$(PREFIX)/bin @@ -316,6 +316,7 @@ TESTOBJ = test/fixture.o \ test/testpreprocessor.o \ test/testprocessexecutor.o \ test/testprogrammemory.o \ + test/testsarif.o \ test/testsettings.o \ test/testsimplifytemplate.o \ test/testsimplifytokens.o \ @@ -834,6 +835,9 @@ test/testprocessexecutor.o: test/testprocessexecutor.cpp cli/executor.h cli/proc test/testprogrammemory.o: test/testprogrammemory.cpp lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/programmemory.h lib/settings.h lib/standards.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h test/helpers.h $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testprogrammemory.cpp +test/testsarif.o: test/testsarif.cpp lib/config.h lib/json.h test/fixture.h test/helpers.h + $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testsarif.cpp + test/testsettings.o: test/testsettings.cpp lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h $(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testsettings.cpp diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 4f0649324f6..2709273f8a2 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -92,33 +92,65 @@ namespace { if (finding.callStack.empty()) continue; if (ruleIds.insert(finding.id).second) { + // setting name and description to empty strings will make github default + // to the instance specific violation message and not rule description, + // this makes it so not all the violations have the same description. picojson::object rule; rule["id"] = picojson::value(finding.id); + // rule.name + rule["name"] = picojson::value(""); // rule.shortDescription.text picojson::object shortDescription; - shortDescription["text"] = picojson::value(finding.shortMessage()); + shortDescription["text"] = picojson::value(""); rule["shortDescription"] = picojson::value(shortDescription); // rule.fullDescription.text picojson::object fullDescription; - fullDescription["text"] = picojson::value(finding.verboseMessage()); + fullDescription["text"] = picojson::value(""); rule["fullDescription"] = picojson::value(fullDescription); // rule.help.text picojson::object help; - help["text"] = picojson::value(finding.verboseMessage()); // FIXME provide proper help text + help["text"] = picojson::value(""); rule["help"] = picojson::value(help); // rule.properties.precision, rule.properties.problem.severity picojson::object properties; properties["precision"] = picojson::value(sarifPrecision(finding)); - const char* securitySeverity = nullptr; - if (finding.severity == Severity::error && !ErrorLogger::isCriticalErrorId(finding.id)) - securitySeverity = "9.9"; // We see undefined behavior - //else if (finding.severity == Severity::warning) - // securitySeverity = 5.1; // We see potential undefined behavior - if (securitySeverity) { - properties["security-severity"] = picojson::value(securitySeverity); - const picojson::array tags{picojson::value("security")}; + // rule.properties.security-severity, rule.properties.tags + picojson::array tags; + + // If we have a CWE ID, treat it as security-related (CWE is the authoritative source for security weaknesses) + if (finding.cwe.id > 0) { + double securitySeverity = 0; + if (finding.severity == Severity::error && !ErrorLogger::isCriticalErrorId(finding.id)) { + securitySeverity = 9.9; // critical = 9.0+ + } + else if (finding.severity == Severity::warning) { + securitySeverity = 8.5; // high = 7.0 to 8.9 + } + else if (finding.severity == Severity::performance || finding.severity == Severity::portability || + finding.severity == Severity::style) { + securitySeverity = 5.5; // medium = 4.0 to 6.9 + } + else if (finding.severity == Severity::information || finding.severity == Severity::internal || + finding.severity == Severity::debug || finding.severity == Severity::none) { + securitySeverity = 2.0; // low = 0.1 to 3.9 + } + if (securitySeverity > 0.0) { + std::ostringstream ss; + ss << securitySeverity; + properties["security-severity"] = picojson::value(ss.str()); + tags.emplace_back("external/cwe/cwe-" + std::to_string(finding.cwe.id)); + tags.emplace_back("security"); + } + } + + // Add tags array if it has any content + if (!tags.empty()) { properties["tags"] = picojson::value(tags); } + + // Set problem.severity for use with github + const std::string problemSeverity = sarifSeverity(finding); + properties["problem.severity"] = picojson::value(problemSeverity); rule["properties"] = picojson::value(properties); // rule.defaultConfiguration.level picojson::object defaultConfiguration; @@ -198,14 +230,15 @@ namespace { return picojson::value(doc).serialize(true); } - private: + private: static std::string sarifSeverity(const ErrorMessage& errmsg) { if (ErrorLogger::isCriticalErrorId(errmsg.id)) return "error"; switch (errmsg.severity) { case Severity::error: case Severity::warning: + return "error"; case Severity::style: case Severity::portability: case Severity::performance: @@ -672,19 +705,21 @@ void StdLogger::reportErr(const ErrorMessage &msg) mGuidelineMapping, msgCopy.severity); msgCopy.classification = getClassification(msgCopy.guideline, mSettings.reportType); - // TODO: there should be no need for verbose and default messages here - const std::string msgStr = msgCopy.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation); + if (mSettings.outputFormat == Settings::OutputFormat::sarif) { + mSarifReport.addFinding(std::move(msgCopy)); + } else { + // TODO: there should be no need for verbose and default messages here + const std::string msgStr = msgCopy.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation); - // Alert only about unique errors - if (!mSettings.emitDuplicates && !mShownErrors.insert(msgStr).second) - return; + // Alert only about unique errors + if (!mSettings.emitDuplicates && !mShownErrors.insert(msgStr).second) + return; - if (mSettings.outputFormat == Settings::OutputFormat::sarif) - mSarifReport.addFinding(std::move(msgCopy)); - else if (mSettings.outputFormat == Settings::OutputFormat::xml) - reportErr(msgCopy.toXML()); - else - reportErr(msgStr); + if (mSettings.outputFormat == Settings::OutputFormat::xml) + reportErr(msgCopy.toXML()); + else + reportErr(msgStr); + } } /** diff --git a/runformat b/runformat index 60ad1b2abe1..5b01926c99d 100755 --- a/runformat +++ b/runformat @@ -14,8 +14,8 @@ UNCRUSTIFY_VERSION="0.72.0" UNCRUSTIFY="${UNCRUSTIFY-uncrustify}" -DETECTED_VERSION=$("$UNCRUSTIFY" --version 2>&1 | grep -o -E '[0-9.]+') -if [ "$DETECTED_VERSION" != "${UNCRUSTIFY_VERSION}" ]; then +DETECTED_VERSION=$("$UNCRUSTIFY" --version 2>&1 | grep -o -E '[0-9]+\.[0-9]+\.[0-9]+') +if [[ "$DETECTED_VERSION" != "${UNCRUSTIFY_VERSION}" ]]; then echo "You should use version: ${UNCRUSTIFY_VERSION}" echo "Detected version: ${DETECTED_VERSION}" exit 1 diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 0bbcea9d8a9..8044af1c249 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -20,6 +20,7 @@ if (BUILD_TESTS) target_include_directories(testrunner SYSTEM PRIVATE ${tinyxml2_INCLUDE_DIRS}) endif() target_externals_include_directories(testrunner PRIVATE ${PROJECT_SOURCE_DIR}/externals/simplecpp/) + target_externals_include_directories(testrunner PRIVATE ${PROJECT_SOURCE_DIR}/externals/picojson/) if (Boost_FOUND) target_include_directories(testrunner SYSTEM PRIVATE ${Boost_INCLUDE_DIRS}) endif() diff --git a/test/cli/helloworld_test.py b/test/cli/helloworld_test.py index 54de0d60930..3b1e21fe19f 100644 --- a/test/cli/helloworld_test.py +++ b/test/cli/helloworld_test.py @@ -377,6 +377,9 @@ def test_sarif(): assert 'security' in res['runs'][0]['tool']['driver']['rules'][0]['properties']['tags'] assert re.match(r'[0-9]+(.[0-9]+)+', res['runs'][0]['tool']['driver']['semanticVersion']) assert 'level' in res['runs'][0]['tool']['driver']['rules'][0]['defaultConfiguration'] # #13885 + assert res['runs'][0]['tool']['driver']['rules'][0]['shortDescription']['text'] == '' + assert res['runs'][0]['results'][0]['message']['text'] == 'Division by zero.' + assert res['runs'][0]['tool']['driver']['rules'][0]['properties']['problem.severity'] == 'error' def test_xml_checkers_report(): diff --git a/test/testsarif.cpp b/test/testsarif.cpp new file mode 100644 index 00000000000..9915d20e872 --- /dev/null +++ b/test/testsarif.cpp @@ -0,0 +1,1111 @@ +/* + * Cppcheck - A tool for static C/C++ code analysis + * Copyright (C) 2007-2025 Cppcheck team. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "fixture.h" +#include "helpers.h" +#include "json.h" + +#include +#include +#include +#include +#include + +// Cross-platform includes for process execution +#ifdef _WIN32 +#include +#include +#define popen _popen +#define pclose _pclose +#else +#include +#include +#endif + +class TestSarif : public TestFixture +{ +public: + TestSarif() : TestFixture("TestSarif") + {} + +private: + // Helper function to check if a string starts with a prefix (C++17 compatible) + static bool startsWith(const std::string& str, const std::string& prefix) { + return str.size() >= prefix.size() && str.compare(0, prefix.size(), prefix) == 0; + } + // Shared test code with various error types + const std::string testCode = R"( +#include +#include +#include +#include +#include +#include + +class TestClass { +public: + TestClass() : value(0) {} + ~TestClass() { delete ptr; } + + void setValue(int v) { value = v; } + int getValue() const { return value; } + +private: + int value; + int* ptr = nullptr; +}; + +void testSecurityViolations() { + // Null pointer dereference + int* ptr = nullptr; + *ptr = 5; + + // Array bounds violation + int array[5]; + array[10] = 1; + + // Memory leak + int* mem = (int*)malloc(sizeof(int) * 10); + // forgot to free mem + + // Uninitialized variable + int x; + printf("%d", x); + + // Double free + int* p = (int*)malloc(sizeof(int)); + free(p); + free(p); + + // Buffer overflow with strcpy + char buffer[10]; + char source[20] = "This is too long"; + strcpy(buffer, source); + + // Integer overflow + int large = 2147483647; + int overflow = large + 1; + + // Use after free + int* freed = (int*)malloc(sizeof(int)); + free(freed); + *freed = 42; + + // Format string vulnerability + char userInput[] = "%s%s%s"; + printf(userInput); +} + +void testStyleAndPortabilityIssues() { + // Redundant assignment + int redundant = 5; + redundant = redundant; + + // Unused variable + int unused = 42; + + // Variable scope reduction + int i; + for (i = 0; i < 10; i++) { + // i could be declared in for loop + } + + // Const correctness + TestClass obj; + obj.getValue(); // should be const + + // C-style cast (prefer static_cast) + double d = 3.14; + int cStyleCast = (int)d; + + // Increment in condition + int counter = 0; + while (++counter < 10) { + // prefer pre-increment outside condition + } + + // Comparison of bool with integer + bool flag = true; + if (flag == 1) { + // prefer if(flag) + } + + // Assignment in condition + int result; + if (result = getValue()) { + // should be == + } + + // Inefficient string concatenation + std::string str = "Hello"; + str = str + " World"; + + // Suspicious semicolon + for (int j = 0; j < 5; j++); + { + printf("This block runs only once\n"); + } +} + +int getValue() { + return 42; +} + +void testErrorHandling() { + // File operations without error checking + FILE* file = fopen("nonexistent.txt", "r"); + fgetc(file); // potential null pointer + + // Division by zero + int zero = 0; + int result = 10 / zero; + + // Missing return statement + // (This function should return int but doesn't always) +} + +void testSTLIssues() { + std::vector vec; + + // Out of bounds access + vec[0] = 1; // vector is empty + + // Iterator invalidation + for (auto it = vec.begin(); it != vec.end(); ++it) { + if (*it == 0) { + vec.push_back(1); // invalidates iterator + } + } + + // Inefficient vector usage + std::vector v(1000); + for (int i = 0; i < 1000; i++) { + v.push_back(i); // should use resize or reserve + } +} + +void testFunctionIssues() { + // Too many arguments (this will trigger complexFunction) + complexFunction(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12); + + // Recursive function without base case + recursiveWithoutBase(5); +} + +void complexFunction(int a, int b, int c, int d, int e, int f, + int g, int h, int i, int j, int k, int l) { + // Function with too many parameters + printf("%d\n", a + b + c + d + e + f + g + h + i + j + k + l); +} + +void recursiveWithoutBase(int n) { + printf("%d\n", n); + recursiveWithoutBase(n - 1); // infinite recursion +} + +// Unused function +static void unusedFunction() { + // This function is never called +} + +// Function with unused parameter +void functionWithUnusedParam(int used, int unused) { + printf("%d\n", used); +} + +// Missing override specifier +class Base { +public: + virtual void virtualFunc() {} + virtual ~Base() = default; +}; + +class Derived : public Base { +public: + void virtualFunc() {} // should have override +}; + +int main() { + testSecurityViolations(); + testStyleAndPortabilityIssues(); + testErrorHandling(); + testSTLIssues(); + testFunctionIssues(); + + functionWithUnusedParam(1, 2); + + return 0; +} + )"; + + void run() override + { + TEST_CASE(sarifBasicStructure); + TEST_CASE(sarifGeneratedOutput); + TEST_CASE(sarifRuleDescriptions); + TEST_CASE(sarifSecuritySeverity); + TEST_CASE(sarifLocationInfo); + TEST_CASE(sarifGenericDescriptions); + TEST_CASE(sarifInstanceSpecificMessages); + TEST_CASE(sarifCweTags); + TEST_CASE(sarifRuleCoverage); + TEST_CASE(sarifSeverityLevels); + TEST_CASE(sarifSecurityRules); + } + + // Cross-platform approach: Use conditional compilation for Windows/Mac/Linux compatibility + // On Windows: Uses _popen/_pclose (mapped via #define) + // On Unix/Linux/Mac: Uses popen/pclose + // This avoids the need for complex process spawning APIs while maintaining compatibility + static std::string runCppcheckSarif(const std::string& code) + { + // Create temporary file + const std::string filename = "sarif_test_temp.cpp"; + std::ofstream file(filename); + file << code; + file.close(); + + try + { + // Try multiple possible locations for the cppcheck executable + std::vector possibleExes = { + "./cppcheck", + "../bin/cppcheck", + "../../bin/cppcheck", + "./cmake.output/bin/cppcheck", + "../cmake.output/bin/cppcheck", + "cppcheck" // fallback to PATH + }; + + std::string exe; + for (const auto& possibleExe : possibleExes) { + // Test if executable exists and is accessible +#ifdef _WIN32 + std::string testCmd = "\"" + possibleExe + "\" --version >nul 2>&1"; +#else + std::string testCmd = possibleExe + " --version >/dev/null 2>&1"; +#endif + if (system(testCmd.c_str()) == 0) { + exe = possibleExe; + break; + } + } + + if (exe.empty()) { + std::remove(filename.c_str()); + return ""; + } + + std::string args = "--output-format=sarif --enable=all " + filename; + +#ifdef _WIN32 + // Windows-specific command building with proper quoting + std::string cmd = "\"" + exe + " " + args + "\" 2>&1"; +#else + // Unix/Linux/Mac command building + std::string cmd = exe + " " + args + " 2>&1"; +#endif + + // Use cross-platform popen (mapped to _popen on Windows via #define) + FILE* pipe = popen(cmd.c_str(), "r"); + if (!pipe) + { + std::remove(filename.c_str()); + return ""; + } + + std::string result; + char buffer[1024]; + while (fgets(buffer, sizeof(buffer), pipe) != nullptr) + { + result += buffer; + } + + // Use cross-platform pclose (mapped to _pclose on Windows via #define) + pclose(pipe); + + // Clean up temporary file + std::remove(filename.c_str()); + + // Extract just the JSON part (skip "Checking..." line) + size_t jsonStart = result.find('{'); + if (jsonStart != std::string::npos) + { + return result.substr(jsonStart); + } + return ""; + } + catch (...) + { + // Clean up on any exception + std::remove(filename.c_str()); + return ""; + } + } + + // Helper to parse and validate SARIF JSON + static bool validateSarifJson(const std::string& sarifOutput, std::string& errorMsg) + { + if (sarifOutput.empty()) + { + errorMsg = "Empty SARIF output"; + return false; + } + + picojson::value json; + std::string parseError = picojson::parse(json, sarifOutput); + if (!parseError.empty()) + { + errorMsg = "JSON parse error: " + parseError; + return false; + } + + if (!json.is()) + { + errorMsg = "Root is not an object"; + return false; + } + + const picojson::object& root = json.get(); + + // Check required SARIF fields + if (root.find("version") == root.end() || !root.at("version").is()) + { + errorMsg = "Missing or invalid version field"; + return false; + } + + if (root.find("$schema") == root.end() || !root.at("$schema").is()) + { + errorMsg = "Missing or invalid $schema field"; + return false; + } + + if (root.find("runs") == root.end() || !root.at("runs").is()) + { + errorMsg = "Missing or invalid runs field"; + return false; + } + + const picojson::array& runs = root.at("runs").get(); + if (runs.empty()) + { + errorMsg = "Empty runs array"; + return false; + } + + return true; + } + + void sarifBasicStructure() + { + // Create a simple test with null pointer dereference + const std::string basicTestCode = R"( + int main() { + int* p = nullptr; + *p = 5; // null pointer dereference + return 0; + } + )"; + + const std::string sarif = runCppcheckSarif(basicTestCode); + + std::string errorMsg; + const bool isValid = validateSarifJson(sarif, errorMsg); + if (!isValid) + { + // Print for debugging + std::cout << "SARIF Output: " << sarif << std::endl; + std::cout << "Error: " << errorMsg << std::endl; + } + ASSERT_EQUALS(true, isValid); + + // Parse and check specific SARIF structure + picojson::value json; + picojson::parse(json, sarif); + const picojson::object& root = json.get(); + + ASSERT_EQUALS("2.1.0", root.at("version").get()); + ASSERT(root.at("$schema").get().find("sarif-schema") != std::string::npos); + + const picojson::array& runs = root.at("runs").get(); + ASSERT_EQUALS(1, static_cast(runs.size())); + + const picojson::object& sarifRun = runs[0].get(); + ASSERT(sarifRun.find("tool") != sarifRun.end()); + ASSERT(sarifRun.find("results") != sarifRun.end()); + + const picojson::object& tool = sarifRun.at("tool").get(); + ASSERT(tool.find("driver") != tool.end()); + + const picojson::object& driver = tool.at("driver").get(); + ASSERT_EQUALS("Cppcheck", driver.at("name").get()); + ASSERT(driver.find("rules") != driver.end()); + } + + void sarifGeneratedOutput() + { + const std::string sarif = runCppcheckSarif(testCode); + + std::string errorMsg; + ASSERT_EQUALS(true, validateSarifJson(sarif, errorMsg)); + + // Parse and check for different error types + picojson::value json; + picojson::parse(json, sarif); + const picojson::object& root = json.get(); + const picojson::array& runs = root.at("runs").get(); + const picojson::object& sarifRun = runs[0].get(); + const picojson::array& results = sarifRun.at("results").get(); + + ASSERT(!results.empty()); + + // Check that we have different severity levels and meaningful messages + bool hasError = false; + bool hasNonEmptyMessage = false; + + for (const auto& result : results) + { + const picojson::object& res = result.get(); + const std::string level = res.at("level").get(); + if (level == "error") + hasError = true; + + // Verify each result has a meaningful message + ASSERT(res.find("message") != res.end()); + const picojson::object& message = res.at("message").get(); + ASSERT(message.find("text") != message.end()); + const std::string messageText = message.at("text").get(); + + // Messages should be non-empty and meaningful + ASSERT(!messageText.empty()); + if (messageText.length() > 5) // Reasonable minimum for a meaningful message + { + hasNonEmptyMessage = true; + } + + // Basic validation that messages don't contain obvious placeholders + ASSERT_EQUALS(std::string::npos, messageText.find("{{")); + ASSERT_EQUALS(std::string::npos, messageText.find("}}")); + } + + ASSERT_EQUALS(true, hasError); + ASSERT_EQUALS(true, hasNonEmptyMessage); + } + + void sarifRuleDescriptions() + { + const std::string sarif = runCppcheckSarif(testCode); + + picojson::value json; + picojson::parse(json, sarif); + const picojson::object& root = json.get(); + const picojson::array& runs = root.at("runs").get(); + const picojson::object& sarifRun = runs[0].get(); + const picojson::object& tool = sarifRun.at("tool").get(); + const picojson::object& driver = tool.at("driver").get(); + const picojson::array& rules = driver.at("rules").get(); + + ASSERT(!rules.empty()); + + // Verify that ALL rule descriptions are empty so GitHub uses instance-specific messages + for (const auto& rule : rules) + { + const picojson::object& r = rule.get(); + const std::string name = r.at("name").get(); + + // Verify we have proper rule structure + ASSERT(r.find("shortDescription") != r.end()); + ASSERT(r.find("fullDescription") != r.end()); + + const picojson::object& shortDesc = r.at("shortDescription").get(); + const std::string shortText = shortDesc.at("text").get(); + + const picojson::object& fullDesc = r.at("fullDescription").get(); + const std::string fullText = fullDesc.at("text").get(); + + // The key test: ALL rule descriptions should be empty + // This allows GitHub to automatically use instance-specific messages + ASSERT_EQUALS("", name); + ASSERT_EQUALS("", shortText); + ASSERT_EQUALS("", fullText); + } + } + + void sarifSecuritySeverity() + { + const std::string sarif = runCppcheckSarif(testCode); + + picojson::value json; + picojson::parse(json, sarif); + const picojson::object& root = json.get(); + const picojson::array& runs = root.at("runs").get(); + const picojson::object& sarifRun = runs[0].get(); + const picojson::object& tool = sarifRun.at("tool").get(); + const picojson::object& driver = tool.at("driver").get(); + const picojson::array& rules = driver.at("rules").get(); + + // Check that security-related rules have security-severity + bool foundSecurityRule = false; + for (const auto& rule : rules) + { + const picojson::object& r = rule.get(); + const std::string ruleId = r.at("id").get(); + + if (ruleId == "nullPointer" || ruleId == "arrayIndexOutOfBounds" || ruleId == "doubleFree" || + ruleId == "memleak" || ruleId == "uninitvar") + { + const picojson::object& props = r.at("properties").get(); + if (props.find("security-severity") != props.end()) + { + foundSecurityRule = true; + const std::string secSev = props.at("security-severity").get(); + ASSERT(std::stod(secSev) > 0.0); + + // Should also have tags + if (props.find("tags") != props.end()) + { + const picojson::array& tags = props.at("tags").get(); + bool hasSecurityTag = false; + bool hasCweTag = false; + for (const auto& tag : tags) + { + const std::string& tagStr = tag.get(); + if (tagStr == "security") + { + hasSecurityTag = true; + } + else if (startsWith(tagStr, "external/cwe/cwe-")) + { + hasCweTag = true; + } + } + ASSERT_EQUALS(true, hasSecurityTag); + ASSERT_EQUALS(true, hasCweTag); + } + } + } + } + + ASSERT_EQUALS(true, foundSecurityRule); + } + + void sarifLocationInfo() + { + const std::string sarif = runCppcheckSarif(testCode); + + picojson::value json; + picojson::parse(json, sarif); + const picojson::object& root = json.get(); + const picojson::array& runs = root.at("runs").get(); + const picojson::object& sarifRun = runs[0].get(); + const picojson::array& results = sarifRun.at("results").get(); + + ASSERT(!results.empty()); + + // Check location information + for (const auto& result : results) + { + const picojson::object& res = result.get(); + + ASSERT(res.find("locations") != res.end()); + const picojson::array& locations = res.at("locations").get(); + ASSERT(!locations.empty()); + + for (const auto& location : locations) + { + const picojson::object& loc = location.get(); + ASSERT(loc.find("physicalLocation") != loc.end()); + + const picojson::object& physLoc = loc.at("physicalLocation").get(); + ASSERT(physLoc.find("artifactLocation") != physLoc.end()); + ASSERT(physLoc.find("region") != physLoc.end()); + + const picojson::object& region = physLoc.at("region").get(); + ASSERT(region.find("startLine") != region.end()); + ASSERT(region.find("startColumn") != region.end()); + + // Line numbers should be positive, columns should be positive (SARIF requires >= 1) + if (region.at("startLine").is()) + { + const int64_t line = static_cast(region.at("startLine").get()); + ASSERT(line > 0); + } + + if (region.at("startColumn").is()) + { + const int64_t col = static_cast(region.at("startColumn").get()); + ASSERT(col > 0); + } + } + } + } + + void sarifGenericDescriptions() + { + const std::string sarif = runCppcheckSarif(testCode); + + picojson::value json; + picojson::parse(json, sarif); + const picojson::object& root = json.get(); + const picojson::array& runs = root.at("runs").get(); + const picojson::object& sarifRun = runs[0].get(); + const picojson::object& tool = sarifRun.at("tool").get(); + const picojson::object& driver = tool.at("driver").get(); + const picojson::array& rules = driver.at("rules").get(); + + ASSERT(!rules.empty()); + + // Verify that ALL rule descriptions are empty so GitHub uses instance-specific messages + for (const auto& rule : rules) + { + const picojson::object& r = rule.get(); + const std::string name = r.at("name").get(); + + // Verify we have proper rule structure + ASSERT(r.find("shortDescription") != r.end()); + ASSERT(r.find("fullDescription") != r.end()); + + const picojson::object& shortDesc = r.at("shortDescription").get(); + const std::string shortText = shortDesc.at("text").get(); + + const picojson::object& fullDesc = r.at("fullDescription").get(); + const std::string fullText = fullDesc.at("text").get(); + + // The key test: ALL rule descriptions should be empty + // This allows GitHub to automatically use instance-specific messages + ASSERT_EQUALS("", name); + ASSERT_EQUALS("", shortText); + ASSERT_EQUALS("", fullText); + } + } + + void sarifInstanceSpecificMessages() + { + // Use the global testCode to validate instance-specific messages + const std::string sarif = runCppcheckSarif(testCode); + + std::string errorMsg; + ASSERT_EQUALS(true, validateSarifJson(sarif, errorMsg)); + + picojson::value json; + picojson::parse(json, sarif); + const picojson::object& root = json.get(); + const picojson::array& runs = root.at("runs").get(); + const picojson::object& sarifRun = runs[0].get(); + const picojson::array& results = sarifRun.at("results").get(); + + ASSERT(!results.empty()); + + // Validate that results contain instance-specific information + bool foundArrayBoundsWithSpecifics = false; + bool foundAnyInstanceSpecificMessage = false; + + for (const auto& result : results) + { + const picojson::object& res = result.get(); + const std::string ruleId = res.at("ruleId").get(); + const picojson::object& message = res.at("message").get(); + const std::string messageText = message.at("text").get(); + + // Skip system include warnings as they're not relevant to our test + if (ruleId == "missingIncludeSystem") + continue; + + if (ruleId == "arrayIndexOutOfBounds") + { + // Should contain specific array name and/or index from testCode + if ((messageText.find("array") != std::string::npos && messageText.find("10") != std::string::npos) || + (messageText.find("Array") != std::string::npos && messageText.find("10") != std::string::npos)) + { + foundArrayBoundsWithSpecifics = true; + foundAnyInstanceSpecificMessage = true; + // Verify it's about array bounds + ASSERT(messageText.find("bound") != std::string::npos || + messageText.find("index") != std::string::npos || + messageText.find("Array") != std::string::npos); + } + } + else if (ruleId == "nullPointer") + { + // Should contain the specific variable name from our test (ptr) + if (messageText.find("ptr") != std::string::npos) + { + foundAnyInstanceSpecificMessage = true; + // Verify it's a meaningful message about null pointer dereference + ASSERT(messageText.find("null") != std::string::npos || + messageText.find("nullptr") != std::string::npos || + messageText.find("NULL") != std::string::npos || + messageText.find("Null") != std::string::npos); + } + } + else if (ruleId == "memleak") + { + // Should contain the specific variable name from testCode (mem) + if (messageText.find("mem") != std::string::npos) + { + foundAnyInstanceSpecificMessage = true; + // Verify it's about memory leak + ASSERT(messageText.find("leak") != std::string::npos || + messageText.find("free") != std::string::npos || + messageText.find("memory") != std::string::npos); + } + } + else if (ruleId == "uninitvar") + { + // Should contain the specific variable name from testCode (x) + if (messageText.find("'x'") != std::string::npos) + { + foundAnyInstanceSpecificMessage = true; + // Verify it's about uninitialized variable + ASSERT(messageText.find("uninit") != std::string::npos || + messageText.find("initial") != std::string::npos); + } + } + else if (ruleId == "unusedVariable") + { + // Should contain specific variable names from testCode (unused, redundant, etc.) + if (messageText.find("unused") != std::string::npos || + messageText.find("redundant") != std::string::npos || + messageText.find("counter") != std::string::npos) + { + foundAnyInstanceSpecificMessage = true; + // Verify it's about unused variable + ASSERT(messageText.find("unused") != std::string::npos || + messageText.find("never used") != std::string::npos); + } + } + else if (ruleId == "doubleFree") + { + // Should contain specific variable name from testCode (p) + if (messageText.find("'p'") != std::string::npos) + { + foundAnyInstanceSpecificMessage = true; + // Verify it's about double free + ASSERT(messageText.find("free") != std::string::npos || + messageText.find("deallocat") != std::string::npos); + } + } + else if (ruleId == "redundantAssignment") + { + // Should contain specific variable name from testCode (redundant) + if (messageText.find("redundant") != std::string::npos) + { + foundAnyInstanceSpecificMessage = true; + // Verify it's about redundant assignment + ASSERT(messageText.find("redundant") != std::string::npos || + messageText.find("assign") != std::string::npos); + } + } + + // Verify that all messages are non-empty and meaningful + ASSERT(!messageText.empty()); + + // Messages should not contain generic placeholders + ASSERT_EQUALS(std::string::npos, messageText.find("{{")); + ASSERT_EQUALS(std::string::npos, messageText.find("}}")); + } + + // We must find at least the array bounds violation with specific details + ASSERT_EQUALS(true, foundArrayBoundsWithSpecifics); + + // We should find at least one instance-specific message overall + ASSERT_EQUALS(true, foundAnyInstanceSpecificMessage); + } + + void sarifCweTags() + { + const std::string sarif = runCppcheckSarif(testCode); + + picojson::value json; + picojson::parse(json, sarif); + const picojson::object& root = json.get(); + const picojson::array& runs = root.at("runs").get(); + const picojson::object& sarifRun = runs[0].get(); + const picojson::object& tool = sarifRun.at("tool").get(); + const picojson::object& driver = tool.at("driver").get(); + const picojson::array& rules = driver.at("rules").get(); + + // Check that rules with CWE IDs have CWE tags (regardless of security classification) + bool foundNullPointerWithCwe = false; + bool foundArrayBoundsWithCwe = false; + bool foundMemleakWithCwe = false; + bool foundDoubleFreeWithCwe = false; + bool foundAnyCweTag = false; + + for (const auto& rule : rules) + { + const picojson::object& r = rule.get(); + const std::string ruleId = r.at("id").get(); + const picojson::object& props = r.at("properties").get(); + + // Check if this rule has tags + if (props.find("tags") != props.end()) + { + const picojson::array& tags = props.at("tags").get(); + + bool hasSecurityTag = false; + bool hasCweTag = false; + + for (const auto& tag : tags) + { + const std::string& tagStr = tag.get(); + if (tagStr == "security") + { + hasSecurityTag = true; + } + else if (startsWith(tagStr, "external/cwe/cwe-")) + { + hasCweTag = true; + foundAnyCweTag = true; + + // Validate CWE tag format: external/cwe/cwe- + ASSERT_EQUALS(0, tagStr.find("external/cwe/cwe-")); + std::string cweNumber = tagStr.substr(17); // After "external/cwe/cwe-" + ASSERT(!cweNumber.empty()); + + // Verify it's a valid number + for (char c : cweNumber) + { + ASSERT(c >= '0' && c <= '9'); + } + + // Track specific CWE mappings we expect for security-related rules + if (ruleId == "nullPointer" && cweNumber == "476") + foundNullPointerWithCwe = true; + else if (ruleId == "arrayIndexOutOfBounds" && cweNumber == "788") + foundArrayBoundsWithCwe = true; + else if (ruleId == "memleak" && cweNumber == "401") + foundMemleakWithCwe = true; + else if (ruleId == "doubleFree" && cweNumber == "415") + foundDoubleFreeWithCwe = true; + } + } + + // If this is a security-related rule with CWE, it should have both security and CWE tags + if (hasCweTag && (ruleId == "nullPointer" || ruleId == "arrayIndexOutOfBounds" || + ruleId == "doubleFree" || ruleId == "memleak")) + { + // Security-related rules should have security tag when they have security-severity + if (props.find("security-severity") != props.end()) + { + ASSERT_EQUALS(true, hasSecurityTag); + } + } + } + } + + // Verify we found at least some CWE tags (from any rules, not just security-related) + ASSERT_EQUALS(true, foundAnyCweTag); + + // Verify we found at least some of the expected security-related CWE mappings + // Note: Not all may be present depending on what the test code triggers + bool foundSecurityCweMapping = + foundNullPointerWithCwe || foundArrayBoundsWithCwe || foundMemleakWithCwe || foundDoubleFreeWithCwe; + ASSERT_EQUALS(true, foundSecurityCweMapping); + } + + void sarifRuleCoverage() + { + const std::string sarif = runCppcheckSarif(testCode); + + picojson::value json; + picojson::parse(json, sarif); + const picojson::object& root = json.get(); + const picojson::array& runs = root.at("runs").get(); + const picojson::object& sarifRun = runs[0].get(); + const picojson::object& tool = sarifRun.at("tool").get(); + const picojson::object& driver = tool.at("driver").get(); + const picojson::array& rules = driver.at("rules").get(); + + // Verify we have a good variety of rules triggered + std::set ruleIds; + for (const auto& rule : rules) + { + const picojson::object& r = rule.get(); + const std::string ruleId = r.at("id").get(); + ruleIds.insert(ruleId); + } + + // We should have at least 10 different rules triggered by our comprehensive test + ASSERT(ruleIds.size() >= 10); + + // Check for some specific expected rules from different categories + std::vector expectedRules = { + "nullPointer", // Security + "arrayIndexOutOfBounds", // Security + "memleak", // Security + "uninitvar", // Security + "unusedVariable", // Style + "redundantAssignment", // Style + "unusedFunction", // Style + "constParameter", // Style/Performance + "cstyleCast", // Style + "variableScope" // Style + }; + + int foundExpectedRules = std::count_if(expectedRules.begin(), expectedRules.end(), + [&ruleIds](const std::string& expectedRule) { + return ruleIds.find(expectedRule) != ruleIds.end(); + }); + + // We should find at least half of our expected rules + ASSERT(foundExpectedRules >= static_cast(expectedRules.size() / 2)); + } + + void sarifSeverityLevels() + { + const std::string sarif = runCppcheckSarif(testCode); + + picojson::value json; + picojson::parse(json, sarif); + const picojson::object& root = json.get(); + const picojson::array& runs = root.at("runs").get(); + const picojson::object& sarifRun = runs[0].get(); + const picojson::array& results = sarifRun.at("results").get(); + + ASSERT(!results.empty()); + + // Track different severity levels + bool hasError = false; + bool hasWarning = false; + bool hasNote = false; + + for (const auto& result : results) + { + const picojson::object& res = result.get(); + const std::string level = res.at("level").get(); + + if (level == "error") + hasError = true; + else if (level == "warning") + hasWarning = true; + else if (level == "note") + hasNote = true; + } + + // Our comprehensive test should trigger multiple severity levels + ASSERT_EQUALS(true, hasError); + // We should have at least one non-error level + ASSERT(hasWarning || hasNote); + + // Verify rule consistency between rules and results + const picojson::object& tool = sarifRun.at("tool").get(); + const picojson::object& driver = tool.at("driver").get(); + const picojson::array& rules = driver.at("rules").get(); + + std::set ruleIdsInRules; + std::set ruleIdsInResults; + + for (const auto& rule : rules) + { + const picojson::object& r = rule.get(); + ruleIdsInRules.insert(r.at("id").get()); + } + + for (const auto& result : results) + { + const picojson::object& res = result.get(); + ruleIdsInResults.insert(res.at("ruleId").get()); + } + + // Every rule ID in results should have a corresponding rule definition + for (const std::string& resultRuleId : ruleIdsInResults) + { + ASSERT(ruleIdsInRules.find(resultRuleId) != ruleIdsInRules.end()); + } + } + + void sarifSecurityRules() + { + const std::string sarif = runCppcheckSarif(testCode); + + picojson::value json; + picojson::parse(json, sarif); + const picojson::object& root = json.get(); + const picojson::array& runs = root.at("runs").get(); + const picojson::object& sarifRun = runs[0].get(); + const picojson::object& tool = sarifRun.at("tool").get(); + const picojson::object& driver = tool.at("driver").get(); + const picojson::array& rules = driver.at("rules").get(); + + // Verify that security classification is correctly based on CWE IDs + // Rules with CWE IDs should have security properties, rules without should not + bool foundRuleWithCWE = false; + bool foundRuleWithoutCWE = false; + + for (const auto& rule : rules) + { + const picojson::object& r = rule.get(); + const picojson::object& props = r.at("properties").get(); + + // Check if rule has CWE tag + bool hasCWE = false; + if (props.find("tags") != props.end()) + { + const picojson::array& tags = props.at("tags").get(); + hasCWE = std::any_of(tags.begin(), tags.end(), [](const picojson::value& tag) { + return startsWith(tag.get(), "external/cwe/"); + }); + } + + if (hasCWE) + { + foundRuleWithCWE = true; + // Rules with CWE should have security-severity and security tag + ASSERT(props.find("security-severity") != props.end()); + + // Check for security tag + bool hasSecurityTag = false; + if (props.find("tags") != props.end()) + { + const picojson::array& tags = props.at("tags").get(); + hasSecurityTag = std::any_of(tags.begin(), tags.end(), [](const picojson::value& tag) { + return tag.get() == "security"; + }); + } + ASSERT(hasSecurityTag); + } + else + { + foundRuleWithoutCWE = true; + // Rules without CWE should NOT have security-severity or security tag + ASSERT(props.find("security-severity") == props.end()); + + if (props.find("tags") != props.end()) + { + const picojson::array& tags = props.at("tags").get(); + for (const auto& tag : tags) + { + const std::string& tagStr = tag.get(); + ASSERT(tagStr != "security"); + } + } + } + + // All rules should still have basic properties + ASSERT(props.find("precision") != props.end()); + ASSERT(props.find("problem.severity") != props.end()); + } + + // We should find at least some rules in our test data + ASSERT(foundRuleWithCWE || foundRuleWithoutCWE); + } +}; + +REGISTER_TEST(TestSarif)