GitHub fixes (#1580)

prioux · web-flow · commit 2d765cba30be · 2025-11-25T17:34:34.000-05:00
Changes relating to GitHub code scanners.

The code scanners reported a bunch of small issues, none
of them critical. Fixes here. Sometimes, it's just
writing the code in a slighly different way.
diff --git a/.github/workflows/cbrain_ci.yaml b/.github/workflows/cbrain_ci.yaml
@@ -20,6 +20,9 @@ jobs:
     runs-on: ubuntu-24.04
     env:
       RAILS_ENV: test
+    permissions:
+      contents: read
+      pull-requests: read
 
     ###########################################################
     services:
diff --git a/Bourreau/spec/boutiques/boutiques_tester_spec.rb b/Bourreau/spec/boutiques/boutiques_tester_spec.rb
@@ -392,7 +392,7 @@
       @task.user_id, @task.group_id = UID, GID
       # Generate a simulated exit file, as if the task had run
       @simExitFile     = @task.exit_cluster_filename
-      IO.write( @simExitFile, "0\n" )
+      File.write( @simExitFile, "0\n" )
       # The basic properties for the required output file
       @reqOutfileProps = {:name => @fname_base, :data_provider_id => @provider.id}
       # Optional output file properties
@@ -428,11 +428,11 @@
         expect( @task.save_results ).to be false
       end
       it "save_results is false if the exit status file has invalid content" do
-        IO.write( @simExitFile, "abcde\n" )
+        File.write( @simExitFile, "abcde\n" )
         expect( @task.save_results ).to be false
       end
       it "save_results is false if the exit status file contains a value greater than 1" do
-        IO.write( @simExitFile, "3\n" )
+        File.write( @simExitFile, "3\n" )
         expect( @task.save_results ).to be false
       end
 
diff --git a/BrainPortal/app/controllers/portal_controller.rb b/BrainPortal/app/controllers/portal_controller.rb
@@ -424,16 +424,6 @@ def report #:nodoc:
   def search
     @search  = params[:search]
     @limit   = 20 # used by interface only
-
-    # In development mode, classes are loaded at first use. This means a dev
-    # will sometimes NOT see a class (e.g. TextFile) until first use, which means
-    # that some parts of the interface will not show them. This trick allows a dev
-    # to force the load of a class just by typing the name in the search box.
-    # The string HAS to be something like 'TextFile' or 'TarArchive' etc.
-    if Rails.env == 'development' && @search.present? && @search.to_s =~ /\A[A-Z]\w+\z/
-      eval @search.to_s rescue nil  # just load a class, if needed
-    end
-
     @results = @search.present? ? ModelsReport.search_for_token(@search, current_user) : {}
   end
 
diff --git a/BrainPortal/app/controllers/quotas_controller.rb b/BrainPortal/app/controllers/quotas_controller.rb
@@ -395,10 +395,10 @@ def base_scope #:nodoc:
   # Tries to turn strings like '3 mb' into 3_000_000 etc.
   # Supported suffixes are T, G, M, K, TB, GB, MB, KB, B (case insensitive).
   def guess_size_units(sizestring)
-    match = sizestring.match(/\A\s*(-?\d*\.?\d+)\s*([tgmk]?)\s*b?\s*\z/i)
+    match = sizestring.match(/\A\s*(-?\d{1,5}(\.\d{1,2})?)\s*([tgmk]?)\s*b?\s*\z/i)
     return "" unless match # parsing error
     number = match[1]
-    suffix = match[2].presence&.downcase || 'u'
+    suffix = match[3].presence&.downcase || 'u'
     mult   = { 't' => 1_000_000_000_000, 'g' => 1_000_000_000, 'm' => 1_000_000, 'k' => 1_000, 'u' => 1 }
     totbytes = number.to_f * mult[suffix]
     totbytes = totbytes.to_i
@@ -409,10 +409,10 @@ def guess_size_units(sizestring)
   # Supported suffixes are s, h, d, m, w, and y (case insensitive).
   # Minutes not supported because of the sad existance of months.
   def guess_time_units(timestring)
-    match = timestring.match(/\A\s*(\d*\.?\d+)\s*([shdwmy]?)\s*\z/i)
+    match = timestring.match(/\A\s*(\d{1,4}(\.\d{1,2})?)\s*([shdwmy]?)\s*\z/i)
     return "" unless match # parsing error
     number = match[1]
-    suffix = match[2].presence&.downcase || 's'
+    suffix = match[3].presence&.downcase || 's'
     mult   = { 's' => 1.second, 'h' => 1.hour,  'd' => 1.day,
                'w' => 1.week,   'm' => 1.month, 'y' => 1.year, }
     tottime = number.to_f * mult[suffix].to_i
diff --git a/BrainPortal/app/controllers/userfiles_controller.rb b/BrainPortal/app/controllers/userfiles_controller.rb
@@ -370,7 +370,7 @@ def display
 
     # No viewer
     if ! @viewer
-      render :html => "<div class=\"warning\">Could not find viewer #{viewer_name}.</div>".html_safe, :status  => "404"
+      render :html => "<div class=\"warning\">Could not find viewer #{ERB::Util.html_escape(viewer_name || '(Unset)')}.</div>".html_safe, :status  => "404"
       return
     end
 
@@ -401,7 +401,7 @@ def display
       :description => "An internal error occurred when trying to display the contents of #{@userfile.name}."
     )
 
-    render :html => "<div class=\"warning\">Error generating view code for viewer '#{params[:viewer]}'. Admins have been notified and will look into the problem. In the meantime, there's not much you can do about this.</div>".html_safe
+    render :html => "<div class=\"warning\">Error generating view code for viewer '#{ERB::Util.html_escape(params[:viewer] || '(Unset)')}'. Admins have been notified and will look into the problem. In the meantime, there's not much you can do about this.</div>".html_safe
   end
 
   def show #:nodoc:
diff --git a/BrainPortal/app/models/boutiques_portal_task.rb b/BrainPortal/app/models/boutiques_portal_task.rb
@@ -532,7 +532,7 @@ def sanitize_param(input)
     # Some presets for convenience; at most one 'if' will trigger because regex != string always
     charset_regex  = /\A[\w,\.\:\-]+\z/                       if charset_regex == ':basename:'        # "a0_,.:-"
     charset_regex  = /\A[\w,\.\:\-\?\*]+\z/                   if charset_regex == ':basename-pattern:'  # "a0_,.:-*?"
-    charset_regex  = /\A[\w,\.\/\:\-]+(\/[\w,\.\/\:\-]*)*\z/  if charset_regex == ':relative-path:'   # "base" or "/base/base/..."
+    charset_regex  = /\A[\w,\.\/\:\-]+(\/[\w,\.\:\-]+)*\/?\z/ if charset_regex == ':relative-path:'   # "base" or "/base/base/..."
     charset_regex  = /\A\S+\z/                                if charset_regex == ':any-no-blanks:'   # can be dangerous! YOU MUST VALIDATE TOOL'S ESCAPING PROPERLY!
     charset_regex  = /\A[\w,\.\:\-\{\}]+\z/                   if charset_regex == ':id-with-curlies:' # allows "abc" and "abc-{4}" etc
     charset_regex  = /\A[\w,\.\:\-\+]+(\ +[\w,\.\:\-\+]+)*\z/ if charset_regex == ':ids-with-spaces:' # allows "abc" and "abc def xyz" etc
diff --git a/BrainPortal/app/models/help_document.rb b/BrainPortal/app/models/help_document.rb
@@ -56,7 +56,7 @@ def full_path
 
   # Pseudo-attribute representing the document's contents
   def contents
-    @contents ||= (File.file?(self.full_path) ? IO.read(self.full_path) : nil)
+    @contents ||= (File.file?(self.full_path) ? File.read(self.full_path) : nil)
   end
 
   def contents=(contents) #:nodoc:
@@ -79,7 +79,7 @@ def self.from_existing_file!(key, path = nil)
 
     # FIXME Inefficient; the file is re-written in the before_save callback.
     doc          = self.new(:key => key, :path => path);
-    doc.contents = IO.read(doc.full_path)
+    doc.contents = File.read(doc.full_path)
     doc.save!
     doc
   end
@@ -99,7 +99,7 @@ def write_doc
 
     if @contents
       FileUtils.mkpath(doc_dir) unless File.file?(doc_path) || File.directory?(doc_dir)
-      IO.write(doc_path, @contents)
+      File.write(doc_path, @contents)
     else
       File.unlink(doc_path) if File.file?(doc_path)
     end
diff --git a/BrainPortal/app/models/task_custom_filter.rb b/BrainPortal/app/models/task_custom_filter.rb
@@ -175,6 +175,7 @@ def scope_types(scope)
   def scope_description(scope)
     query = 'cbrain_tasks.description'
     term = self.data_description_term
+    term = "do-not-match-everything-#{rand(1000000)}" if term =~ /\A[\%\_\s]+\z/ # don't try matching all
     if self.data_description_type == 'match'
       query += ' = ?'
     else
diff --git a/BrainPortal/app/models/userfile.rb b/BrainPortal/app/models/userfile.rb
@@ -106,7 +106,7 @@ class Userfile < ApplicationRecord
   attr_accessor           :sync_select_patterns
 
   # Utility named scopes
-  scope :name_like,     -> (n) { where("userfiles.name LIKE ?", "%#{n.strip}%") }
+  scope :name_like,     -> (n) { where("userfiles.name LIKE ? ESCAPE '!'", "%#{n.strip.gsub(/([%_!])/,'!\1')}%") }
 
   scope :has_no_parent, ->     { where(parent_id: nil) }
 
diff --git a/BrainPortal/app/models/userfile_custom_filter.rb b/BrainPortal/app/models/userfile_custom_filter.rb
@@ -207,6 +207,7 @@ def filter_scope(scope)
   def scope_name(scope)
     query = 'userfiles.name'
     term = self.data_file_name_term
+    term = "do-not-match-everything-#{rand(1000000)}" if term =~ /\A[\%\_]+\z/ # don't try matching all
     if self.data_file_name_type == 'match'
       query += ' = ?'
     else
diff --git a/BrainPortal/app/views/userfiles/index.csv.erb b/BrainPortal/app/views/userfiles/index.csv.erb
@@ -20,22 +20,24 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 -%>
-<% delimiter    = params[:delimiter] || "," -%>
-<%-
-# Should escape the quote_marker in description.
-# Maybe buggy when quote_marker is not a double quote.
+<%
+   delimiter    = params[:delimiter] || ','
+   quote_marker = params[:quote]     || '"'
+   delimiter    = ',' unless %w( , | ; ! - + # ).include?(delimiter)
+   quote_marker = '"' unless %w( ' " ).include?(quote_marker)
+   rows = @userfiles.map do |u|
+      [ u.name,
+        u.type,
+        u.user&.login,
+        u.group&.name,
+        to_localtime(u.created_at, :datetime),
+        to_localtime(u.updated_at, :datetime),
+        u.data_provider&.name,
+        u.tags.map(&:name).join(','),
+        u.description.presence || ""
+      ]
+   end
 -%>
-<% quote_marker = params[:quote]     || "\"" -%>
-<% @userfiles.each do |u| -%>
-<%= [quote_marker + u.name                                + quote_marker,
-       u.size,
-       quote_marker + u.type                                + quote_marker,
-       quote_marker + u.user.try(:login)                    + quote_marker,
-       quote_marker + u.group.try(:name)                    + quote_marker,
-       quote_marker + to_localtime(u.created_at, :datetime) + quote_marker,
-       quote_marker + to_localtime(u.updated_at, :datetime) + quote_marker,
-       quote_marker + u.data_provider.try(:name)            + quote_marker,
-       quote_marker + u.tags.map(&:name).join(delimiter)    + quote_marker,
-       quote_marker + u.description.to_s.gsub(/\"/, "\"\"") + quote_marker,
-      ].join(delimiter).html_safe %>
-<% end -%>
+<%= CSV.generate(:force_quotes => true, :col_sep => delimiter, :quote_char => quote_marker) do |csv|
+      rows.each { |r| csv << r }
+    end.html_safe -%>
diff --git a/BrainPortal/cbrain_plugins/cbrain-plugins-base/boutiques_descriptors/tool_configurator.json b/BrainPortal/cbrain_plugins/cbrain-plugins-base/boutiques_descriptors/tool_configurator.json
@@ -98,7 +98,7 @@
             "name":              "Build Image Mode",
             "id":                "build_mode",
             "type":              "String",
-            "description":       "The source for the Apptainer image that will be assigned to the NEW ToolConfig. The image can be build from Docker, or just copied if an existing image exists in the OLD ToolConfig. When building, use 'docker-daemon' if there is a local docker system, and the tool will pull to it before building the Apptainer image. When using 'docker', Apptainer will pull directly for DockerHub instead.",
+            "description":       "The source for the Apptainer image that will be assigned to the NEW ToolConfig. The image can be build from Docker, or just copied if an existing image exists in the OLD ToolConfig. When building, use 'docker-daemon' if there is a local docker system, and the tool will pull to it before building the Apptainer image. When using 'docker', Apptainer will pull directly for DockerHub instead. Please click any of the 'Refresh' buttons above if you change this value, so as to auto update the fields below.",
             "optional":          false,
             "list":              false,
             "value-key":         "[BUILD_MODE]",
@@ -187,6 +187,10 @@
         "code": 1,
         "description": "Build script error"
       },
+      {
+        "code": 2,
+        "description": "Apptainer Build command probably ran out of memory"
+      },
       {
         "code": 22,
         "description": "Docker inspect command failed"
diff --git a/BrainPortal/lib/cbrain_task_generators/schema_task_generator.rb b/BrainPortal/lib/cbrain_task_generators/schema_task_generator.rb
@@ -298,11 +298,11 @@ def to_directory(path)
       Dir.chdir(path) do
         ['portal', 'bourreau', 'views/public'].each { |d| FileUtils.mkpath(d) }
 
-        IO.write("portal/#{name}.rb",                  @source[:portal])
-        IO.write("bourreau/#{name}.rb",                @source[:bourreau])
-        IO.write("views/_task_params.html.erb",        @source[:task_params])
-        IO.write("views/_show_params.html.erb",        @source[:show_params])
-        IO.write("views/public/edit_params_help.html", @source[:edit_help])
+        File.write("portal/#{name}.rb",                  @source[:portal])
+        File.write("bourreau/#{name}.rb",                @source[:bourreau])
+        File.write("views/_task_params.html.erb",        @source[:task_params])
+        File.write("views/_show_params.html.erb",        @source[:show_params])
+        File.write("views/public/edit_params_help.html", @source[:edit_help])
       end
     end
 
@@ -363,7 +363,7 @@ def self.generate(schema, descriptorInput, strict_validation = true, file_for_re
     end
 
     apply_template = lambda do |template|
-      ERB.new(IO.read(
+      ERB.new(File.read(
         Rails.root.join('lib/cbrain_task_generators/templates', template).to_s
       ), nil, '%-').result(binding)
     end
@@ -548,7 +548,7 @@ def self.default_schema
   def self.expand_json(obj)
     return obj unless obj.is_a?(String)
 
-    JSON.parse!(File.exists?(obj) ? IO.read(obj) : obj)
+    JSON.parse!(File.exists?(obj) ? File.read(obj) : obj)
   end
 
   # Utility method to convert a string (+str+) to an identifier suitable for a
diff --git a/BrainPortal/lib/http_user_agent.rb b/BrainPortal/lib/http_user_agent.rb
@@ -75,9 +75,10 @@ def parse(user_agent_string)
     keyvals = {}
     lastname = ""
     adj_ua.split(/[\s;()]+/).each do |comp|
-      next unless comp =~ /\A(\S+)\/(\S+)\z/
-      name  = Regexp.last_match[1]
-      value = Regexp.last_match[2]
+      comps = comp.split("/")          # "Super/Mega/Chrome/1.23"
+      next unless comps.size >= 2
+      value = comps.pop                # "1.23"
+      name  = comps.join("/")          # "Super/Mega/Chrome"
       next if keyvals.has_key?(name.downcase) # first match has priority
       keyvals[name.downcase] = value
       lastname = name
diff --git a/BrainPortal/lib/models_report.rb b/BrainPortal/lib/models_report.rb
@@ -151,9 +151,9 @@ def self.rr_usage_statistics(options)
   def self.search_for_token(token, user=current_user) #:nodoc:
 
     token       = token.to_s.presence || "-9998877"          # -9998877 is a way to ensure we find nothing ...
+    token.strip!
     is_numeric  = token =~ /\A\d+\z/  || token == "-9998877" # ... because we'll find by ID
 
-
     file_scope  = Userfile      .find_all_accessible_by_user(user, :access_requested => :read).order(:name)
     task_scope  = CbrainTask    .find_all_accessible_by_user(user) .order(:id)
     rr_scope    = RemoteResource.find_all_accessible_by_user(user) .order(:name)
@@ -180,6 +180,7 @@ def self.search_for_token(token, user=current_user) #:nodoc:
         :tcs    => Array(tc_scope    .find_by_id(token)) ,
       }
     else
+      token = "do-not-match-everything-#{rand(1000000)}" if token =~ /\A[\%\_]+\z/ # don't try matching all
       ptoken = "%#{token}%"
       {
         # Use a wide window to edit this code! Keep it clean!
diff --git a/BrainPortal/lib/neurohub_helpers.rb b/BrainPortal/lib/neurohub_helpers.rb
@@ -133,6 +133,7 @@ def nh_service_storages(user)
   def neurohub_search(token, limit=20, user=current_user)
     token      = token.to_s.presence  || "-9998877"          # -9998877 is a way to ensure we find nothing ...
     is_numeric = token =~ /\A\d+\z/   || token == "-9998877" # ... because we'll find by ID
+    token      = "do-not-match-everything-#{rand(1000000)}" if token =~ /\A[\%\_]*\z/ # don't try matching all
     token      = is_numeric ? token.to_i : "%#{token}%"
 
     if is_numeric
diff --git a/BrainPortal/lib/oidc_config.rb b/BrainPortal/lib/oidc_config.rb
@@ -125,7 +125,7 @@ def self.find_by_name(name)
   # recover during the protocol negotiation the proper OidcConfig
   # we're using.
   def create_state(session_id_string)
-    state = Digest::MD5.hexdigest( session_id_string ) + "_" + self.name
+    state = Digest::SHA256.hexdigest( session_id_string ) + "_" + self.name
     return state # just to be clear
   end
 
@@ -135,8 +135,8 @@ def self.find_by_state(state)
     # Verify state structure is 33 hex chars + "_" + oidc_name
     # and extract name
     oidc_name = ""
-    if state.length >= 34 && state[32] == '_'
-      oidc_name = state[33..-1]
+    if state.length >= 66 && state[64] == '_'
+      oidc_name = state[65..-1]
     end
 
     self.find_by_name(oidc_name)
diff --git a/BrainPortal/lib/subpath_format_validator.rb b/BrainPortal/lib/subpath_format_validator.rb
@@ -26,7 +26,7 @@ class SubpathFormatValidator < ActiveModel::EachValidator #:nodoc:
 
   def validate_each(object, attribute, value) #:nodoc:
     # FIXME currently, hidden files and directories are not supported
-    unless value.blank? || value =~ /\A(?:[^.\/][^\/]*\/?)+\z/
+    unless value.blank? || value =~ /\A[^\.\/]+(\/[^\/]+)*\z/
       object.errors[attribute] << (options[:message] || "contains invalid characters")
     end
   end
