Beta version of Security feedback in Sigrid CI

.github/workflows/sigrid-pullrequest.yml

@@ -15,7 +15,7 @@ jobs:
       - name: "Run Sigrid CI" 
         env:
           SIGRID_CI_TOKEN: "${{ secrets.SIGRID_CI_TOKEN }}"
-        run: "./sigridci/sigridci.py --customer sig --system sigridci-client --source . --capability maintainability,osh"
+        run: "./sigridci/sigridci.py --customer sig --system sigridci-client --source . --capability maintainability,osh,security"
       - name: "Save Sigrid CI results"
         if: always()
         uses: actions/upload-artifact@v4
@@ -44,6 +44,6 @@ jobs:
         with:
           customer: "sig"
           system: "sigridci-client"
-          capability: "maintainability,osh"
+          capability: "maintainability,osh,security"
         env:
           SIGRID_CI_TOKEN: "${{ secrets.SIGRID_CI_TOKEN }}"

docs/reference/client-script-usage.md

@@ -28,7 +28,7 @@ The script takes a limited number of mandatory arguments. However, Sigrid CI's b
 | `--system`                    | Yes      | examplesystemname   | Name of your system in Sigrid. Contact SIG support if you are not sure about this. [2]                                            |
 | `--subsystem `                | No       | frontend            | Used to map between repository directory structure versus the one known by Sigrid. [5]                                            |
 | `--source`                    | No       | .                   | Path of your project's source code. Use "." for current directory.                                                                |
-| `--capability`                | No       | maintainability     | Comma-separated list of Sigrid capabilities (`maintainability,osh`). Default is maintainability.                                  |
+| `--capability`                | No       | maintainability     | Comma-separated list of Sigrid capabilities (`maintainability,osh,security`). Default is maintainability.                         |
 | `--publish`                   | No       | N/A                 | Automatically publishes analysis results to Sigrid. [1]                                                                           |
 | `--publishonly`               | No       | N/A                 | Publishes analysis results to Sigrid, but *does not* provide feedback in the CI environment itself. [3]                           |
 | `--exclude`                   | No       | /build/,.png        | Comma-separated list of file and/or directory names that should be excluded from the upload. [4, 7]                               |

docs/sigridci-integration/using-sigridci.md

@@ -98,6 +98,37 @@ Sigrid CI separates vulnerable open source libraries into two categories:
   line `message-path: sigrid-ci-output/feedback.md`, and change this to `message-path: sigrid-ci-output/*feedback.md`.
   Adding the asterisk allows you to get feedback on *all* Sigrid capabilities, not just maintainability.
 
+### Security feedback (Beta)
+
+Sigrid CI provides security feedback based on your [objectives](../capabilities/portfolio-objectives.md).
+As with [open source vulnerabilities](#open-source-health-feedback-beta), you do need to fix every single
+security finding that does not meet your objective.
+
+<img src="../images/ci/security-feedback.png" width="350" />
+
+When you encounter security findings during code reviews, there are three ways how you can deal with them:
+
+- **Address the finding in the pull request:** It's always the best course of action to just address the finding
+  within the pull request. This prevents the finding from ever going into the main/master branch, which is generally
+  considered a best practice in [shift-left thinking](https://en.wikipedia.org/wiki/Shift-left_testing). 
+- **Merge the pull request, manage the finding via Sigrid:** In some cases, the pull request author and reviewer might
+  agree it's not feasible to address the finding right now. In those situations, it's OK to merge the pull request.
+  This will cause the security finding to appear in Sigrid's
+  [Security dashboard](../capabilities/portfolio-security.md), where it can be tracked.
+- **Merge the pull request, mark the finding as a false positive in Sigrid:** Like any automated check, Sigrid can
+  produce findings that are false positives. In those situations, if the pull request author and reviewer agree the
+  finding is *actually* a false positive, it's OK to merge the pull request. You can then mark the finding as a false
+  positive in Sigrid's [security page](../capabilities/system-security.md). False positives are automatically excluded
+  from future Sigrid CI feedback. 
+
+#### Adding Security feedback to an existing Sigrid CI configuration
+
+- **All platforms:** You need to add the option `--capability maintainability,osh,security` to the Sigrid CI step in
+  your pipeline configuration.
+- **GitHub:** In addition to the above, you need one extra step: In your pipeline configuration, look for the
+  line `message-path: sigrid-ci-output/feedback.md`, and change this to `message-path: sigrid-ci-output/*feedback.md`.
+  Adding the asterisk allows you to get feedback on *all* Sigrid capabilities, not just maintainability.
+
 ## How do you deal with feedback from Sigrid CI?
 
 Feedback from Sigrid CI is intended to be used in the context of a

sigridci/sigridci.py

@@ -18,17 +18,14 @@
 import sys
 from argparse import ArgumentParser, SUPPRESS
 
-from sigridci.capability import MAINTAINABILITY, OPEN_SOURCE_HEALTH
+from sigridci.capability import CAPABILITY_SHORT_NAMES
 from sigridci.publish_options import PublishOptions, RunMode
 from sigridci.sigrid_api_client import SigridApiClient
 from sigridci.platform import Platform
 from sigridci.sigridci_runner import SigridCiRunner
 from sigridci.upload_log import UploadLog
 
 
-CAPABILITIES = {cap.shortName: cap for cap in [MAINTAINABILITY, OPEN_SOURCE_HEALTH]}
-
-
 def parsePublishOptions(args):
     return PublishOptions(
         partner=args.partner.lower(),
@@ -66,7 +63,7 @@ def parseTarget(target):
 
 def parseCapabilities(names):
     try:
-        return [CAPABILITIES[name.lower().strip()] for name in names.split(",")]
+        return [CAPABILITY_SHORT_NAMES[name.lower().strip()] for name in names.split(",")]
     except KeyError as e:
         print(f"Invalid value for --capability: {str(e)}")
         sys.exit(1)
@@ -81,7 +78,8 @@ def parseCapabilities(names):
     parser.add_argument("--subsystem", type=str, default="", help="Publishes your code as a subsystem within a Sigrid system.")
     parser.add_argument("--convert", type=str, default="", help="Code conversion for specific technologies")
     parser.add_argument("--source", type=str, required=True, help="Path of your project's source code.")
-    parser.add_argument("--capability", type=str, default="maintainability", help=f"Comma-separated Sigrid capabilities ({','.join(CAPABILITIES.keys())}).")
+    parser.add_argument("--capability", type=str, default="maintainability",
+                        help=f"Comma-separated Sigrid capabilities ({','.join(CAPABILITY_SHORT_NAMES.keys())}).")
     parser.add_argument("--publish", action="store_true", help="Publishes analysis results to Sigrid.")
     parser.add_argument("--publishonly", action="store_true", help="Only publishes to Sigrid without waiting for results.")
     parser.add_argument("--exclude", type=str, default="", help="Comma-separated list of files/directories to exclude.")

sigridci/sigridci/analysisresults/findings_processor.py

@@ -0,0 +1,76 @@
+# Copyright Software Improvement Group
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from dataclasses import dataclass
+
+from ..objective import Objective
+
+
+@dataclass
+class Finding:
+    fingerprint: str
+    risk: str
+    description: str
+    file: str
+    line: int
+    partOfObjective: bool
+
+
+class FindingsProcessor:
+    def extractFindings(self, feedback, objective):
+        if feedback is None:
+            return []
+        elif "runs" in feedback:
+            sarifProcessor = SarifProcessor()
+            return list(sarifProcessor.extractFindings(feedback, objective))
+        else:
+            sigridFindingsProcessor = SigridFindingsProcessor()
+            return list(sigridFindingsProcessor.extractFindings(feedback, objective))
+
+
+class SarifProcessor:
+    def extractFindings(self, feedback, objective):
+        rules = list(self.getRules(feedback))
+
+        for run in feedback["runs"]:
+            for result in run.get("results", []):
+                fingerprint = result["fingerprints"]["sigFingerprint/v1"]
+                risk = self.getFindingSeverity(result, rules)
+                file = result["locations"][0]["physicalLocation"]["artifactLocation"]["uri"]
+                line = result["locations"][0]["physicalLocation"]["region"]["startLine"]
+                partOfObjective = Objective.isFindingIncluded(risk, objective)
+                yield Finding(fingerprint, risk, result["message"]["text"], file, line, partOfObjective)
+
+    def getRules(self, feedback):
+        for run in feedback["runs"]:
+            for rule in run.get("rules", []):
+                properties = rule.get("properties", {})
+                if properties.get("severity"):
+                    yield rule
+
+    def getFindingSeverity(self, result, rules):
+        severity = result.get("properties", {}).get("severity")
+        if not severity:
+            for rule in rules:
+                if rule["id"] == result["ruleId"]:
+                    severity = rule["properties"]["severity"].replace("ERROR", "HIGH").replace("WARNING", "MEDIUM")
+        return severity.upper() if severity else "UNKNOWN"
+
+
+class SigridFindingsProcessor:
+    def extractFindings(self, feedback, objective):
+        for finding in feedback:
+            partOfObjective = Objective.isFindingIncluded(finding["severity"], objective)
+            yield Finding(finding["id"], finding["severity"], finding["type"],
+                          finding["filePath"], finding["startLine"], partOfObjective)

sigridci/sigridci/capability.py

@@ -27,3 +27,5 @@ class Capability:
 MAINTAINABILITY = Capability("MAINTAINABILITY", "Maintainability", "maintainability", 2, False)
 OPEN_SOURCE_HEALTH = Capability("OPEN_SOURCE_HEALTH", "Open Source Health", "osh", 4, True)
 SECURITY = Capability("SECURITY", "Security", "security", 8, True)
+
+CAPABILITY_SHORT_NAMES = {cap.shortName: cap for cap in [MAINTAINABILITY, OPEN_SOURCE_HEALTH, SECURITY]}

sigridci/sigridci/feedback_provider.py

@@ -26,6 +26,7 @@
 from .reports.osh_text_report import OpenSourceHealthTextReport
 from .reports.pipeline_summary_report import PipelineSummaryReport
 from .reports.security_markdown_report import SecurityMarkdownReport
+from .reports.security_text_report import SecurityTextReport
 from .reports.static_html_report import StaticHtmlReport
 
 
@@ -96,5 +97,7 @@ def prepareAdditionalReports(self, markdownReport):
             reports += [AsciiArtReport(), JUnitFormatReport(), StaticHtmlReport(self.objective)]
         elif self.capability == OPEN_SOURCE_HEALTH:
             reports += [OpenSourceHealthTextReport(self.objective)]
+        elif self.capability == SECURITY:
+            reports += [SecurityTextReport(self.objective)]
         reports.append(PipelineSummaryReport(markdownReport))
         return reports

sigridci/sigridci/platform.py

@@ -16,6 +16,13 @@
 import sys
 
 
+DOCS_URL = f"https://docs.sigrid-says.com"
+SCOPE_DOCS = f"{DOCS_URL}/reference/analysis-scope-configuration.html"
+OSH_EXCLUDE_DOCS = f"{SCOPE_DOCS}#exclude-open-source-health-risks"
+SECURITY_EXCLUDE_RULE_DOCS = f"{SCOPE_DOCS}#excluding-security-rules"
+SECURITY_EXCLUDE_FILE_DOCS = f"{SCOPE_DOCS}#excluding-files-and-directories-from-security-scanning"
+
+
 class Platform:
     @staticmethod
     def isGitHub():

sigridci/sigridci/reports/osh_markdown_report.py

@@ -19,13 +19,13 @@
 from ..analysisresults.cyclonedx_processor import CycloneDXProcessor
 from ..capability import OPEN_SOURCE_HEALTH
 from ..objective import Objective
+from ..platform import OSH_EXCLUDE_DOCS
 
 
 class OpenSourceHealthMarkdownReport(Report, MarkdownRenderer):
     MAX_FINDINGS = SecurityMarkdownReport.MAX_FINDINGS
     SYMBOLS = SecurityMarkdownReport.SEVERITY_SYMBOLS
     SORT_RISK = list(SecurityMarkdownReport.SEVERITY_SYMBOLS.keys())
-    DOCS_LINK = "https://docs.sigrid-says.com/reference/analysis-scope-configuration.html#exclude-open-source-health-risks"
 
     def __init__(self, objective = "HIGH"):
         super().__init__()
@@ -56,7 +56,7 @@ def renderMarkdown(self, analysisId, feedback, options):
             details += "> Consider upgrading to a version that no longer contains the vulnerability.\n\n"
             details += self.generateFindingsTable(fixable, options)
             details += "If you believe these findings are false positives, "
-            details += f"you can [exclude them in the Sigrid configuration]({self.DOCS_LINK}).\n\n"
+            details += f"you can [exclude them in the Sigrid configuration]({OSH_EXCLUDE_DOCS}).\n\n"
         if len(unfixable) > 0:
             details += "## 😑 You have findings that you need to investigate in more depth\n\n"
             details += f"> You have **{len(unfixable)}** vulnerable open source libraries without a fix available.  \n"
@@ -84,7 +84,7 @@ def generateFindingsTable(self, libraries, options):
 
         for library in sorted(libraries, key=lambda lib: self.SORT_RISK.index(lib.risk))[0:self.MAX_FINDINGS]:
             symbol = self.SYMBOLS[library.risk]
-            check = "✅" if Objective.isFindingIncluded(library.risk, self.objective) else "-"
+            check = "✅" if library.partOfObjective else "-"
             locations = "<br />".join(self.decorateLink(options, file, file) for file in library.files)
             md += f"| {symbol} | {check} | {library.name} {library.version} | {library.latestVersion} | {locations} |\n"