feat(sarif): carry every tool's related locations and confidence to the merged log

CHANGELOG.md

@@ -18,6 +18,9 @@ Notable changes are grouped by release. See [all releases](https://github.com/Co
 - **SARIF:** a rule of the merged log carries the title, description and documentation link its
   tool gives, so that GitHub code scanning can explain the finding; flawfinder's rule titles and
   CWE links come first, the analyzers' when they describe their rules.
+- **SARIF:** the related locations and the confidence a tool writes in its SARIF reach the merged
+  log (the runtime analyzer's allocation site, the Python analyzer's sink), and a result's
+  message links to its related locations, which GitHub code scanning then shows as links.
 
 ## v0.85.0
 

CMakeLists.txt

@@ -870,6 +870,12 @@ add_test(NAME ctrace_dyn_findings_merge_with_static_ones
     WORKING_DIRECTORY ${CMAKE_SOURCE_DIR})
 # A finding the runtime analyzer reports without a location keeps none: no file is made up for
 # it, and the run does not fail on it.
+# The allocation of a heap overflow, which the runtime analyzer gives as a related location, and
+# the finding's confidence reach the merged log, whose message links to the allocation.
+set(ctrace_runtime_related_sarif ${CMAKE_BINARY_DIR}/runtime-related.sarif)
+add_test(NAME ctrace_sarif_keeps_the_runtime_allocation_site
+    COMMAND sh -c "rm -f '${ctrace_runtime_related_sarif}'; $<TARGET_FILE:ctrace> --sarif-format --fail-on none --dyn --config ${CMAKE_BINARY_DIR}/fake-runtime-analyzer.json --input tests/runtime/heap_overflow.c --report-file '${ctrace_runtime_related_sarif}' > /dev/null 2>&1; grep -q '\"allocated here\"' '${ctrace_runtime_related_sarif}' && grep -q '(\\[allocated here\\](0))' '${ctrace_runtime_related_sarif}' && grep -q '\"confidence\": \"high\"' '${ctrace_runtime_related_sarif}'"
+    WORKING_DIRECTORY ${CMAKE_SOURCE_DIR})
 file(WRITE ${CMAKE_BINARY_DIR}/fake-runtime-no-location.json
     "{\"schema_version\": 1, \"tools\": {\"coretrace-runtime-analyzer\": {\"path\": \"${CMAKE_SOURCE_DIR}/tests/fake-runtime-analyzer.sh\", \"args\": [\"--fake-no-location\"]}}}\n")
 set(ctrace_no_location_sarif ${CMAKE_BINARY_DIR}/no-location.sarif)

README.md

@@ -362,6 +362,13 @@ runtime analyzers do not describe their rules yet
 [coretrace-runtime-analyzer#29](https://github.com/CoreTrace/coretrace-runtime-analyzer/issues/29)):
 their rules keep their id alone.
 
+A finding keeps the other places it involves and how sure its tool is, whichever tool reports
+it: the related locations (`relatedLocations`, such as the other access of a data race or the
+allocation of a heap overflow), which the message links to so that code scanning shows them as
+links, and the confidence (`properties.confidence`, `low`, `medium` or `high`). The stack
+analyzer's confidence, a number, is not carried yet
+([coretrace-stack-analyzer#244](https://github.com/CoreTrace/coretrace-stack-analyzer/issues/244)).
+
 A weakness reported by several tools, same file, same line and same CWE, is one result: the
 most severe report is kept in its tool's run, and its `alsoReportedBy` property names the other
 tools and rules. Reports without a CWE cannot be matched and are all kept; the console output

include/Process/Tools/InputPaths.hpp

@@ -87,9 +87,10 @@ namespace ctrace
         std::filesystem::path workingDirectory_;
     };
 
-    /// Spells the file of every finding as InputPaths::display() does, for a tool that reports
-    /// the files it was given its own way: cppcheck and tscancode rewrite the paths of their file
-    /// list, the runtime analyzer names its source. Resolves nothing when there is no finding.
+    /// Spells the file of every finding, and of each place it involves, as InputPaths::display()
+    /// does, for a tool that reports the files it was given its own way: cppcheck and tscancode
+    /// rewrite the paths of their file list, the runtime analyzer names its source. Resolves
+    /// nothing when there is no finding.
     inline void respellFindings(std::vector<Diagnostic>& diagnostics,
                                 const std::vector<std::string>& inputs)
     {
@@ -101,6 +102,13 @@ namespace ctrace
         for (Diagnostic& diagnostic : diagnostics)
         {
             diagnostic.file = paths.display(diagnostic.file);
+            for (RelatedLocation& related : diagnostic.relatedLocations)
+            {
+                if (!related.file.empty())
+                {
+                    related.file = paths.display(related.file);
+                }
+            }
         }
     }
 } // namespace ctrace

src/Process/Tools/PythonAnalyzerToolImplementation.cpp

@@ -66,17 +66,25 @@ namespace ctrace
             return diagnostics;
         }
         const InputPaths paths(inputs);
+        // The analyzer locates findings, and the places they involve, relative to the project
+        // root it was given.
+        const auto locate = [&root](const std::string& file)
+        { return fs::path(file).is_relative() ? root / file : fs::path(file); };
         std::vector<Diagnostic> kept;
         for (Diagnostic& diagnostic : *diagnostics)
         {
             if (diagnostic.file.empty())
             {
                 continue;
             }
-            // The analyzer locates findings relative to the project root it was given.
-            const fs::path located = fs::path(diagnostic.file).is_relative()
-                                         ? root / diagnostic.file
-                                         : fs::path(diagnostic.file);
+            for (RelatedLocation& related : diagnostic.relatedLocations)
+            {
+                if (!related.file.empty())
+                {
+                    related.file = paths.display(locate(related.file));
+                }
+            }
+            const fs::path located = locate(diagnostic.file);
             if (auto input = paths.asInput(located))
             {
                 diagnostic.file = *input;

src/Process/Tools/Sarif.cpp

@@ -4,6 +4,7 @@
 #include <nlohmann/json.hpp>
 
 #include <algorithm>
+#include <cctype>
 #include <cstdint>
 #include <filesystem>
 #include <map>
@@ -131,6 +132,95 @@ namespace ctrace
             return described;
         }
 
+        /// The text of a SARIF message without its embedded links (SARIF 2.1.0 §3.11.6): each
+        /// `[text](id)` becomes its text, and the brackets and backslashes escaped around them
+        /// become themselves. The merged log links the message to its related locations again.
+        [[nodiscard]] std::string plainText(std::string_view text)
+        {
+            std::string plain;
+            for (std::size_t at = 0; at < text.size(); ++at)
+            {
+                const char c = text[at];
+                if (c == '\\' && at + 1 < text.size() &&
+                    (text[at + 1] == '\\' || text[at + 1] == '[' || text[at + 1] == ']'))
+                {
+                    plain += text[++at];
+                    continue;
+                }
+                if (c == '[')
+                {
+                    // An embedded link: an unescaped ']' right before '(' and a closing ')'.
+                    std::size_t close = at + 1;
+                    while (close < text.size() && text[close] != ']')
+                        close += text[close] == '\\' ? 2 : 1;
+                    const std::size_t end = close + 1 < text.size() && text[close + 1] == '('
+                                                ? text.find(')', close + 2)
+                                                : std::string_view::npos;
+                    if (close < text.size() && end != std::string_view::npos)
+                    {
+                        plain += plainText(text.substr(at + 1, close - at - 1));
+                        at = end;
+                        continue;
+                    }
+                }
+                plain += c;
+            }
+            return plain;
+        }
+
+        /// The confidence a result's properties give, `low`, `medium` or `high` in any case;
+        /// empty for another value or none.
+        [[nodiscard]] std::string confidenceOf(const nlohmann::json& result)
+        {
+            const auto properties = result.find("properties");
+            if (properties == result.end() || !properties->is_object())
+                return {};
+            const auto confidence = properties->find("confidence");
+            if (confidence == properties->end() || !confidence->is_string())
+                return {};
+            std::string level = confidence->get<std::string>();
+            std::transform(level.begin(), level.end(), level.begin(),
+                           [](unsigned char ch) { return static_cast<char>(std::tolower(ch)); });
+            return level == "low" || level == "medium" || level == "high" ? level : std::string();
+        }
+
+        /// The related locations of a result: each place, when it has one, and its message.
+        /// An entry that is not an object is skipped.
+        [[nodiscard]] std::vector<RelatedLocation> relatedLocationsOf(const nlohmann::json& result)
+        {
+            std::vector<RelatedLocation> related;
+            const auto locations = result.find("relatedLocations");
+            if (locations == result.end() || !locations->is_array())
+                return related;
+            for (const nlohmann::json& location : *locations)
+            {
+                if (!location.is_object())
+                    continue;
+                RelatedLocation place;
+                const nlohmann::json& physical =
+                    location.value("physicalLocation", nlohmann::json::object());
+                if (physical.is_object())
+                {
+                    const nlohmann::json& artifact =
+                        physical.value("artifactLocation", nlohmann::json::object());
+                    const nlohmann::json& region =
+                        physical.value("region", nlohmann::json::object());
+                    if (artifact.is_object())
+                        place.file = artifact.value("uri", "");
+                    if (region.is_object())
+                    {
+                        place.line = region.value("startLine", 0U);
+                        place.column = region.value("startColumn", 0U);
+                    }
+                }
+                const nlohmann::json& message = location.value("message", nlohmann::json::object());
+                if (message.is_object())
+                    place.message = plainText(message.value("text", ""));
+                related.push_back(std::move(place));
+            }
+            return related;
+        }
+
         void readLocation(const nlohmann::json& result, Diagnostic& diagnostic)
         {
             const auto locations = result.find("locations");
@@ -179,8 +269,10 @@ namespace ctrace
                 }
                 diagnostic.severity = severityFromLevel(result.value("level", "warning"));
                 diagnostic.message =
-                    result.value("message", nlohmann::json::object()).value("text", "");
+                    plainText(result.value("message", nlohmann::json::object()).value("text", ""));
                 diagnostic.cwe = cweOf(result, diagnostic.message);
+                diagnostic.confidence = confidenceOf(result);
+                diagnostic.relatedLocations = relatedLocationsOf(result);
                 readLocation(result, diagnostic);
                 diagnostics.push_back(std::move(diagnostic));
             }
@@ -253,6 +345,36 @@ namespace ctrace
             return physical;
         }
 
+        /// `text` as literal SARIF message text, beside embedded links (SARIF 2.1.0 §3.11.6).
+        [[nodiscard]] std::string escaped(std::string_view text)
+        {
+            std::string out;
+            for (const char c : text)
+            {
+                if (c == '\\' || c == '[' || c == ']')
+                    out += '\\';
+                out += c;
+            }
+            return out;
+        }
+
+        /// The message of a result, ending with an embedded link to each related location, which
+        /// GitHub code scanning shows as a link to the place; unchanged without any.
+        [[nodiscard]] std::string linkedMessage(const Diagnostic& diagnostic)
+        {
+            if (diagnostic.relatedLocations.empty())
+                return diagnostic.message;
+            std::string text = escaped(diagnostic.message) + " (";
+            for (std::size_t index = 0; index < diagnostic.relatedLocations.size(); ++index)
+            {
+                if (index != 0)
+                    text += ", ";
+                text += "[" + escaped(diagnostic.relatedLocations[index].message) + "](" +
+                        std::to_string(index) + ")";
+            }
+            return text + ")";
+        }
+
         [[nodiscard]] nlohmann::json resultOf(const Diagnostic& diagnostic)
         {
             nlohmann::json result;
@@ -261,16 +383,18 @@ namespace ctrace
                 result["ruleId"] = diagnostic.ruleId;
             }
             result["level"] = levelOf(diagnostic.severity);
-            result["message"]["text"] = diagnostic.message;
+            result["message"]["text"] = linkedMessage(diagnostic);
             if (!diagnostic.file.empty())
             {
                 result["locations"] = nlohmann::json::array(
                     {{{"physicalLocation",
                        physicalLocationOf(diagnostic.file, diagnostic.line, diagnostic.column)}}});
             }
-            for (const RelatedLocation& related : diagnostic.relatedLocations)
+            for (std::size_t index = 0; index < diagnostic.relatedLocations.size(); ++index)
             {
+                const RelatedLocation& related = diagnostic.relatedLocations[index];
                 nlohmann::json location;
+                location["id"] = index;
                 if (!related.file.empty())
                 {
                     location["physicalLocation"] =

tests/fake-runtime-analyzer.sh

@@ -4,7 +4,7 @@
 #   --format sarif --timeout <seconds> -o <run directory>/<name> [args] -- [compiler args] <source>
 # with the run directory existing and the source last, not starting with `-`, and exits 64
 # otherwise. Then it prints the SARIF the real tool prints for the fixtures, with the source
-# spelled as it was given: a heap overflow for heap_overflow.c, a double free on line 12 for double_free.c, nothing for
+# spelled as it was given, with its confidence: a heap overflow for heap_overflow.c, allocated on line 6, a double free on line 12 for double_free.c, nothing for
 # others. --fake-exit=<code> ends with that code instead (2: could not build or run),
 # --fake-hang never ends, --fake-no-location reports a leak that has no location. When
 # CTRACE_FAKE_RUNTIME_LOG is set, the -o path is appended to it.
@@ -46,7 +46,8 @@ if [ "$location" = no ]; then
 fi
 case "$(basename "$source")" in
     heap_overflow.c) rule=heap-buffer-overflow cwe=CWE-122 line=7 column=15
-        message="heap-buffer-overflow WRITE of size 4" ;;
+        message="heap-buffer-overflow WRITE of size 4"
+        related=', "relatedLocations": [{"id": 0, "message": {"text": "allocated here"}, "physicalLocation": {"artifactLocation": {"uri": "'"$source"'"}, "region": {"startLine": 6, "startColumn": 19}}}]' ;;
     double_free.c) rule=double-free cwe=CWE-415 line=12 column=5 message="double free" ;;
     *) rule= ;;
 esac
@@ -63,10 +64,10 @@ cat <<SARIF
       "ruleId": "$rule",
       "level": "error",
       "message": {"text": "$message"},
-      "properties": {"cwe": "$cwe"},
+      "properties": {"cwe": "$cwe", "confidence": "high"},
       "locations": [{"physicalLocation": {
         "artifactLocation": {"uri": "$source"},
-        "region": {"startLine": $line, "startColumn": $column}}}]
+        "region": {"startLine": $line, "startColumn": $column}}}]${related:-}
     }]
   }]
 }