Output originalBaseUriIds for SARIF report (#1890)

* Output originalBaseUriIds for SARIF report Fixes #1889 * Update SARIF schema to final 2.1.0 version
presidentbeef · Dec 30, 2024 · d834150 · d834150
1 parent ee9de40
commit d834150
Show file tree

Hide file tree

Showing 2 changed files with 228 additions and 5 deletions.
diff --git a/lib/brakeman/report/report_sarif.rb b/lib/brakeman/report/report_sarif.rb
@@ -1,8 +1,10 @@
+require 'uri'
+
 class Brakeman::Report::SARIF < Brakeman::Report::Base
   def generate_report
     sarif_log = {
       :version => '2.1.0',
-      :$schema => 'https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json',
+      :$schema => 'https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0.json',
       :runs => runs,
     }
     JSON.pretty_generate sarif_log
@@ -20,10 +22,122 @@ def runs
           },
         },
         :results => results,
-      },
+      }.merge(original_uri_base_ids)
     ]
   end
 
+  # Output base URIs
+  # based on what the user specified for the application path
+  # and whether or not --absolute-paths was set.
+  def original_uri_base_ids
+    if tracker.options[:app_path] == '.'
+      # Probably no app_path was specified, as that's the default
+
+      if absolute_paths?
+        # Set %SRCROOT% to absolute path
+        {
+          originalUriBaseIds: {
+            '%SRCROOT%' => {
+              uri: file_uri(tracker.app_tree.root),
+              description: {
+                text: 'Base path for application'
+              }
+            }
+          }
+        }
+      else
+        # Empty %SRCROOT%
+        # This avoids any paths appearing in the report
+        # that are not part of the application directory.
+        # Seems fine!
+        {
+          originalUriBaseIds: {
+            '%SRCROOT%' => {
+              description: {
+                text: 'Base path for application'
+              }
+            },
+          }
+        }
+
+      end
+    elsif tracker.options[:app_path] != tracker.app_tree.root
+      # Path was specified and it was relative
+
+      if absolute_paths?
+        # Include absolute root and relative application path
+        {
+          originalUriBaseIds: {
+            PROJECTROOT: {
+              uri: file_uri(tracker.app_tree.root),
+              description: {
+                text: 'Base path for all project files'
+              }
+            },
+            '%SRCROOT%' => {
+              # Technically should ensure this doesn't have any '..'
+              # but... TODO
+              uri: File.join(tracker.options[:app_path], '/'),
+              uriBaseId: 'PROJECTROOT',
+              description: {
+                text: 'Base path for application'
+              }
+            }
+          }
+        }
+      else
+        # Just include relative application path.
+        # Not clear this is 100% valid, but there is one example in the spec like this
+        {
+          originalUriBaseIds: {
+            PROJECTROOT: {
+              description: {
+                text: 'Base path for all project files'
+              }
+            },
+            '%SRCROOT%' => {
+              # Technically should ensure this doesn't have any '..'
+              # but... TODO
+              uri: File.join(tracker.options[:app_path], '/'),
+              uriBaseId: 'PROJECTROOT',
+              description: {
+                text: 'Base path for application'
+              }
+            }
+          }
+        }
+      end
+    else
+      # app_path was absolute
+
+      if absolute_paths?
+        # Set %SRCROOT% to absolute path
+        {
+          originalUriBaseIds: {
+            '%SRCROOT%' => {
+              uri: file_uri(tracker.app_tree.root),
+              description: {
+                text: 'Base path for application'
+              }
+            }
+          }
+        }
+      else
+        # Empty %SRCROOT%
+        # Seems fine!
+        {
+          originalUriBaseIds: {
+            '%SRCROOT%' => {
+              description: {
+                text: 'Base path for application'
+              }
+            },
+          }
+        }
+      end
+    end
+  end
+
   def rules
     @rules ||= unique_warnings_by_warning_code.map do |warning|
       rule_id = render_id warning
@@ -130,4 +244,10 @@ def infer_level warning
     })
     @@levels_from_confidence[warning.confidence]
   end
+
+  # File URI as a string with trailing forward-slash
+  # as required by SARIF standard
+  def file_uri(path)
+    URI::File.build(path: File.join(path, '/')).to_s
+  end
 end
diff --git a/test/tests/sarif_output.rb b/test/tests/sarif_output.rb
@@ -21,16 +21,16 @@ def test_render_message
 
   def test_log_shape
     assert_equal '2.1.0', @@sarif['version']
-    assert_equal 'https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json', @@sarif['$schema']
+    assert_equal 'https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0.json', @@sarif['$schema']
   end
 
   def test_runs_shape
     # Log includes runs, an array, of length 1
     assert runs = @@sarif['runs']
     assert_equal 1, runs.length
 
-    # The single run contains tool, and results
-    assert_equal runs[0].keys, ['tool', 'results']
+    # The single run contains some data
+    assert_equal ['tool', 'results', 'originalUriBaseIds'], runs[0].keys
 
     # The single run contains a single tool
     assert_equal 1, runs[0]['tool'].length
@@ -104,6 +104,17 @@ def test_results_shape
         # Each location has a physical location, ...
         assert location['physicalLocation']
 
+        # Each physical location has an artifact location, ...
+        assert location['physicalLocation']['artifactLocation']
+
+        # Each artifact location has a relative URI
+        assert location['physicalLocation']['artifactLocation']['uri']
+        refute location['physicalLocation']['artifactLocation']['uri'].start_with? 'file://'
+
+
+        # and a uriBaseId
+        assert_equal '%SRCROOT%', location['physicalLocation']['artifactLocation']['uriBaseId']
+
         # Each location has a region
         assert location['physicalLocation']['region']['startLine']
       end
@@ -138,4 +149,96 @@ def test_with_ignore_results_suppression_shape
       assert suppression['justification']
     end
   end
+
+  def test_uri_base_ids_with_absolute_app_path
+    assert base_uris = @@sarif.dig('runs', 0, 'originalUriBaseIds')
+    assert_equal ['%SRCROOT%'], base_uris.keys
+
+    # Only %SRCROOT% with no URI
+    assert base_uris['%SRCROOT%']
+    assert_equal ['description'], base_uris['%SRCROOT%'].keys
+  end
+
+  def test_uri_base_ids_with_relative_app_path
+    original_app_path = tracker_3_2.options[:app_path] # Horrible hack
+    tracker_3_2.options[:app_path] = 'something/relative'
+    sarif = JSON.parse(tracker_3_2.report.to_sarif)
+
+    assert base_uris = sarif.dig('runs', 0, 'originalUriBaseIds')
+    assert_equal ['PROJECTROOT', '%SRCROOT%'], base_uris.keys
+
+    # %SRCROOT% should have the relative path and point to PROJECTROOT
+    # as its base
+    assert base_uris['%SRCROOT%']
+    assert_equal ['uri', 'uriBaseId', 'description'], base_uris['%SRCROOT%'].keys
+    assert_equal 'something/relative/', base_uris['%SRCROOT%']['uri']
+    assert_equal 'PROJECTROOT', base_uris['%SRCROOT%']['uriBaseId']
+
+    # PROJECTROOT should not have a URI
+    assert base_uris['PROJECTROOT']
+    assert_equal ['description'], base_uris['PROJECTROOT'].keys
+  ensure
+    tracker_3_2.options[:app_path] = original_app_path
+  end
+
+  def test_uri_base_ids_with_absolute_app_path_and_absolute_path_option
+    tracker_3_2.options[:absolute_paths] = true
+    sarif = JSON.parse(tracker_3_2.report.to_sarif)
+
+    assert base_uris = sarif.dig('runs', 0, 'originalUriBaseIds')
+    assert_equal ['%SRCROOT%'], base_uris.keys
+
+    # Only %SRCROOT% with absolute URI
+    assert base_uris['%SRCROOT%']
+    assert_equal ['uri','description'], base_uris['%SRCROOT%'].keys
+    assert base_uris['%SRCROOT%']['uri'].start_with? 'file://'
+    assert base_uris['%SRCROOT%']['uri'].end_with? '/'
+  ensure
+    tracker_3_2.options[:absolute_paths] = false
+  end
+
+  def test_uri_base_ids_with_relative_app_path_and_absolute_path_option
+    original_app_path = tracker_3_2.options[:app_path] # Horrible hack
+    tracker_3_2.options[:app_path] = 'something/relative'
+    tracker_3_2.options[:absolute_paths] = true
+    sarif = JSON.parse(tracker_3_2.report.to_sarif)
+
+    assert base_uris = sarif.dig('runs', 0, 'originalUriBaseIds')
+    assert_equal ['PROJECTROOT', '%SRCROOT%'], base_uris.keys
+
+    # %SRCROOT% should have the relative path and point to PROJECTROOT
+    # as its base
+    assert base_uris['%SRCROOT%']
+    assert_equal ['uri', 'uriBaseId', 'description'], base_uris['%SRCROOT%'].keys
+    assert_equal 'something/relative/', base_uris['%SRCROOT%']['uri']
+    assert_equal 'PROJECTROOT', base_uris['%SRCROOT%']['uriBaseId']
+
+    # PROJECTROOT should have an absolute URI
+    assert base_uris['PROJECTROOT']
+    assert_equal ['uri', 'description'], base_uris['PROJECTROOT'].keys
+    assert base_uris['PROJECTROOT']['uri'].start_with? 'file://'
+    assert base_uris['PROJECTROOT']['uri'].end_with? '/'
+  ensure
+    tracker_3_2.options[:app_path] = original_app_path
+    tracker_3_2.options[:absolute_paths] = false
+  end
+
+  def test_uri_base_ids_with_default_app_path_and_absolute_path_option
+    original_app_path = tracker_3_2.options[:app_path] # Horrible hack
+    tracker_3_2.options[:app_path] = '.'
+    tracker_3_2.options[:absolute_paths] = true
+    sarif = JSON.parse(tracker_3_2.report.to_sarif)
+
+    assert base_uris = sarif.dig('runs', 0, 'originalUriBaseIds')
+    assert_equal ['%SRCROOT%'], base_uris.keys
+
+    # Only %SRCROOT% with absolute URI
+    assert base_uris['%SRCROOT%']
+    assert_equal ['uri', 'description'], base_uris['%SRCROOT%'].keys
+    assert base_uris['%SRCROOT%']['uri'].start_with? 'file://'
+    assert base_uris['%SRCROOT%']['uri'].end_with? '/'
+  ensure
+    tracker_3_2.options[:app_path] = original_app_path
+    tracker_3_2.options[:absolute_paths] = false
+  end
 end