Merge pull request #3104 from OSC/update-3.0

Update 3.0

.github/workflows/tests.yml

@@ -43,6 +43,10 @@ jobs:
           bundle install
           gem install rake
 
+      - name: Setup Node.js
+        run: |
+          npm install -g node-gyp
+
       - name: Setup rclone
         run: sudo apt-get update && sudo apt-get install rclone
 

apps/dashboard/app/controllers/files_controller.rb

@@ -2,6 +2,8 @@
 class FilesController < ApplicationController
   include ZipTricks::RailsStreaming
 
+  before_action :strip_sendfile_headers, only: [:fs]
+
   def fs
     request.format = 'json' if request.headers['HTTP_ACCEPT'].split(',').include?('application/json')
 
@@ -52,11 +54,11 @@ def fs
             zip_tricks_stream do |zip|
               @path.files_to_zip.each do |file|
                 begin
-                  next unless File.readable?(file.path)
+                  next unless File.readable?(file.realpath)
 
-                  if File.file?(file.path)
+                  if File.file?(file.realpath)
                     zip.write_deflated_file(file.relative_path.to_s) do |zip_file|
-                      IO.copy_stream(file.path, zip_file)
+                      IO.copy_stream(file.realpath, zip_file)
                     end
                   else
                     zip.add_empty_directory(dirname: file.relative_path.to_s)
@@ -167,6 +169,13 @@ def edit
 
   private
 
+  # set these headers to nil so that we (Rails) will read files
+  # off of disk instead of nginx.
+  def strip_sendfile_headers
+    request.headers['HTTP_X_SENDFILE_TYPE'] = nil
+    request.headers['HTTP_X_ACCEL_MAPPING'] = nil
+  end
+
   def normalized_path(path = params[:filepath])
     Pathname.new("/" + path.to_s.chomp("/").delete_prefix("/"))
   end

apps/dashboard/app/models/active_jobs/jobstatusdata.rb

@@ -118,10 +118,8 @@ def extended_data_slurm(info)
       attributes.push Attribute.new "Total CPUs", info.native[:cpus]
       attributes.push Attribute.new "Time Limit", info.native[:time_limit]
       attributes.push Attribute.new "Time Used", info.native[:time_used]
-      attributes.push Attribute.new "Start Time",
-        DateTime.parse(info.native[:start_time]).strftime("%Y-%m-%d %H:%M:%S") unless info.native[:start_time] == "N/A"
-      attributes.push Attribute.new "End Time",
-        DateTime.parse(info.native[:end_time]).strftime("%Y-%m-%d %H:%M:%S") unless info.native[:end_time] == "N/A"
+      attributes.push Attribute.new "Start Time", safe_parse_time(info.native[:start_time])
+      attributes.push Attribute.new "End Time", safe_parse_time(info.native[:end_time])
       attributes.push Attribute.new "Memory", info.native[:min_memory]
       attributes.push Attribute.new "GRES", info.native[:gres].gsub(/gres:/, "") unless info.native[:gres] == "N/A"
       self.native_attribs = attributes
@@ -305,6 +303,18 @@ def extended_data_default(info)
 
     private
 
+      def safe_parse_time(time)
+        if ['N/A', 'NONE'].include?(time.to_s)
+          ''
+        else
+          begin
+            DateTime.parse(time.to_s).strftime('%Y-%m-%d %H:%M:%S')
+          rescue Date::Error
+            ''
+          end
+        end
+      end
+
       def build_file_explorer_url(path)
         writable_path = (path.writable? ? path : ENV["HOME"]).to_s
 

apps/dashboard/app/models/batch_connect/session.rb

@@ -39,10 +39,6 @@ def get_binding
     # @return [String] session title
     attr_accessor :title
 
-    # The view used to display the connection information for this session
-    # @return [String, nil] session view
-    attr_accessor :view
-
     # Batch connect script type
     # @return [String] script type
     attr_accessor :script_type
@@ -71,13 +67,13 @@ def render_info_view
     # Return the Batch Connect app from the session token
     # @return [BatchConnect::App]
     def app
-      BatchConnect::App.from_token(self.token)
+      @app ||= BatchConnect::App.from_token(self.token)
     end
 
     # Attributes used for serialization
     # @return [Hash] attributes to be serialized
     def attributes
-      %w(id cluster_id job_id created_at token title view script_type cache_completed).map do |attribute|
+      %w(id cluster_id job_id created_at token title script_type cache_completed).map do |attribute|
         [ attribute, nil ]
       end.to_h
     end
@@ -99,6 +95,10 @@ def user_context
       {}
     end
 
+    def view
+      app.session_view
+    end
+
     class << self
       # The data root directory for this namespace
       # @param token [#to_s] The data root directory for a given app token
@@ -236,7 +236,6 @@ def save(app:, context:, format: nil)
       self.id         = SecureRandom.uuid
       self.token      = app.token
       self.title      = app.title
-      self.view       = app.session_view
       self.created_at = Time.now.to_i
       self.cluster_id = context.try(:cluster).to_s
 
@@ -260,15 +259,16 @@ def stage(root, context: nil)
       staged_root.tap { |p| p.mkpath unless p.exist? }
 
       # Sync the template files over
-      oe, s = Open3.capture2e("rsync", "-a", "#{root}/", "#{staged_root}")
+      oe, s = Open3.capture2e('rsync', '-av', '--exclude', '*.erb', "#{root}/", staged_root.to_s)
       raise oe unless s.success?
 
       # Output user submitted context attributes for debugging purposes
       user_defined_context_file.write(JSON.pretty_generate context.as_json)
 
       # Render all template files using ERB
       render_erb_files(
-        template_files,
+        template_files(root),
+        root_dir: root,
         binding: TemplateBinding.new(self, context).get_binding
       )
       true
@@ -465,9 +465,10 @@ def staged_root
     end
 
     # List of template files that need to be rendered
+    # @param dir [#Pathname] the directory where the templates exist
     # @return [Array<Pathname>] list of template files
-    def template_files
-      Pathname.glob(staged_root.join("**", "*.erb")).select(&:file?)
+    def template_files(dir)
+      Pathname.glob(dir.join("**", "*.erb")).select(&:file?)
     end
 
     # Path to script that is sourced before main script is forked
@@ -596,14 +597,20 @@ def job_name
         ].reject(&:blank?).join("/")
       end
 
-      # Render a list of files using ERB
-      def render_erb_files(files, binding: nil, remove_extension: true)
-        files.each do |file|
-          rendered_file = remove_extension ? file.sub_ext("") : file
+      # Render a list of files using ERB. Note the input .erb files are the system
+      # installed erb files (/var/www/ood/...). These are rendered and output into
+      # the staged_root.
+      def render_erb_files(input_files, binding: nil, remove_extension: true, output_dir: staged_root, root_dir: nil)
+        input_files.each do |file|
           template = file.read
-          rendered = ERB.new(template, nil, "-").result(binding)
-          file.rename rendered_file     # keep same file permissions
-          rendered_file.write(rendered)
+          rendered = ERB.new(template, trim_mode: "-").result(binding)
+
+          relative_fname = file.to_s.delete_prefix("#{root_dir}/")
+          output_file = output_dir.join(relative_fname)
+          output_file = remove_extension ? output_file.sub_ext('') : output_file
+
+          output_file.write(rendered)
+          output_file.chmod(file.stat.mode)
         end
       end
   end

apps/dashboard/app/models/posix_file.rb

@@ -1,33 +1,16 @@
 # PosixFile is a class representing a file on a local file system.
 class PosixFile
 
-  attr_reader :path
+  attr_reader :path, :stat
 
-  delegate :basename, :descend, :parent, :join, :to_s, :read, :write, :mkdir, to: :path
+  delegate :basename, :descend, :parent, :join, :to_s, :read, :write, :mkdir, :directory?, :realpath, to: :path
+
+  # include to give us number_to_human_size
+  include ActionView::Helpers::NumberHelper
 
   class << self
     def stat(path)
-      path = Pathname.new(path)
-
-      # path.stat will not work for a symlink and will raise an exception
-      # if the directory or file being pointed at does not exist
-      begin
-        s = path.stat
-      rescue
-        s = path.lstat
-      end
-
-      {
-        id: "dev-#{s.dev}-inode-#{s.ino}",
-        name: path.basename,
-        size: s.directory? ? nil : s.size,
-        human_size: s.directory? ? '-' : ::ApplicationController.helpers.number_to_human_size(s.size, precision: 3),
-        directory: s.directory?,
-        date: s.mtime.to_i,
-        owner: username_from_cache(s.uid),
-        mode: s.mode,
-        dev: s.dev
-      }
+      PosixFile.new(path).to_h
     end
 
     def num_files(from, names)
@@ -59,6 +42,31 @@ def initialize(path)
     # accepts both String and Pathname
     # avoids converting to Pathname in every function
     @path = Pathname.new(path)
+    begin
+      @stat = @path.lstat
+    rescue Errno::ENOENT, Errno::EACCES
+      @stat = nil
+    end
+  end
+
+  def to_h
+    return { name: basename } if stat.nil?
+
+    {
+      id:         "dev-#{stat.dev}-inode-#{stat.ino}",
+      name:       basename,
+      size:       directory? ? nil : stat.size,
+      human_size: human_size,
+      directory:  directory?,
+      date:       stat.mtime.to_i,
+      owner:      PosixFile.username_from_cache(stat.uid),
+      mode:       stat.mode,
+      dev:        stat.dev
+    }
+  end
+
+  def human_size
+    directory? ? '-' : number_to_human_size(stat.size, precision: 3)
   end
 
   def raise_if_cant_access_directory_contents
@@ -68,28 +76,48 @@ def raise_if_cant_access_directory_contents
   end
 
   #FIXME: a more generic name for this?
-  FileToZip = Struct.new(:path, :relative_path)
+  FileToZip = Struct.new(:path, :relative_path, :realpath)
+
+  PATHS_TO_FILTER = ['.', '..'].freeze
 
   def files_to_zip
     expanded = path.expand_path
 
-    expanded.glob('**/*').map { |p|
-      FileToZip.new(p.to_s, p.relative_path_from(expanded).to_s)
-    }
-  end
-
-  def directory?
-    path.stat.directory?
+    expanded.glob('**/*', File::FNM_DOTMATCH).reject do |p|
+      PATHS_TO_FILTER.include?(p.basename.to_s)
+    end.select do |path|
+      AllowlistPolicy.default.permitted?(path.realpath.to_s)
+    end.map do |path|
+      FileToZip.new(path.to_s, path.relative_path_from(expanded).to_s, path.realpath.to_s)
+    end
   end
 
   def ls
     path.each_child.map do |child_path|
-      PosixFile.stat(child_path)
-    end.select do |stats|
-      valid_encoding = stats[:name].to_s.valid_encoding?
-      Rails.logger.warn("Not showing file '#{stats[:name]}' because it is not a UTF-8 filename.") unless valid_encoding
-      valid_encoding
-    end.sort_by { |p| p[:directory] ? 0 : 1 }
+      PosixFile.new(child_path)
+    end.select(&:valid?)
+        .map(&:to_h)
+        .sort_by { |p| p[:directory] ? 0 : 1 }
+  end
+
+  def valid?
+    valid_realpath? && valid_encoding?
+  end
+
+  def valid_realpath?
+    return false if stat.nil? || !path.exist?
+
+    if stat.symlink?
+      AllowlistPolicy.default.permitted?(realpath) && AllowlistPolicy.default.permitted?(path)
+    else
+      AllowlistPolicy.default.permitted?(path)
+    end
+  end
+
+  def valid_encoding?
+    valid = basename.to_s.valid_encoding?
+    Rails.logger.warn("Not showing file '#{stats[:name]}' because it is not a UTF-8 filename.") unless valid
+    valid
   end
 
   def editable?
@@ -125,7 +153,7 @@ def can_download_as_zip?(timeout: Configuration.file_download_dir_timeout, downl
     can_download = false
     error = nil
 
-    if ! (path.directory? && path.readable? && path.executable?)
+    if ! (directory? && path.readable? && path.executable?)
       error = I18n.t('dashboard.files_directory_download_unauthorized')
     else
       # Determine the size of the directory.

apps/dashboard/app/views/layouts/editor.html.erb

@@ -104,6 +104,7 @@
               <option value="elm">Elm</option>
               <option value="erlang">Erlang</option>
               <option value="forth">Forth</option>
+              <option value="fortran">Fortran</option>
               <option value="ftl">FreeMarker</option>
               <option value="gcode">Gcode</option>
               <option value="gherkin">Gherkin</option>

apps/dashboard/config/initializers/upgrade_to_2.1.rb → apps/dashboard/config/initializers/upgrade_to_3.0.rb

@@ -2,11 +2,14 @@
 
 Rails.application.config.after_initialize do
   # Since https://github.com/OSC/ondemand/pull/1526 all the batch connect cache files
-  # have moved. So, when folks upgrade to 2.1, let's sync these old files so that
+  # have moved. So, when folks upgrade to 3.0, let's sync these old files so that
   # they don't lose their cached choices.
   old_context_files = "#{Configuration.dataroot}/batch_connect/**/*/context.json"
   cache_root = BatchConnect::Session.cache_root
 
+  # kick out if you've already done this
+  next if Dir.glob("#{cache_root}/*.json").size.positive?
+
   Dir.glob(old_context_files).map do |old_file|
     new_filename = old_file.gsub(%r{.*/batch_connect/}, '').gsub('/context.json', '').gsub('/', '_')
     new_filename = "#{new_filename}.json"

apps/dashboard/config/initializers/validate_send_files.rb

@@ -0,0 +1,21 @@
+Rails.application.config.after_initialize do
+  module ActionDispatch
+    class Response
+      class FileBody
+
+        # Stream the file's contents if Rack::Sendfile isn't present,
+        # which it isn't for FilesController#fs.
+        def each
+          File.open(to_path, "rb") do |file|
+            real_path = File.readlink("/proc/self/fd/#{file.fileno}")
+            return '' unless AllowlistPolicy.default.permitted?(real_path)
+
+            while chunk = file.read(16_384)
+              yield chunk
+            end
+          end
+        end
+      end
+    end
+  end
+end

apps/dashboard/test/application_system_test_case.rb

@@ -7,6 +7,7 @@ class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
     options.logging_prefs =  { browser: 'ALL' }
   end
 
+  Selenium::WebDriver.logger.level = :debug unless ENV['DEBUG'].nil?
   Capybara.server = :webrick
 
   def find_option_style(ele, opt)

apps/dashboard/test/models/batch_connect/session_test.rb

@@ -526,7 +526,6 @@ def completed?
           'created_at'      => now.to_i,
           'token'           => 'bc_jupyter',
           'title'           => 'Jupyter Notebook',
-          'view'            => nil,
           'script_type'     => 'basic',
           'cache_completed' => nil
         }