Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change "script" to "launcher" in variables #3976

Merged
merged 6 commits into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 33 additions & 33 deletions apps/dashboard/app/controllers/launchers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
class LaunchersController < ApplicationController

before_action :find_project
before_action :find_script, only: [:show, :edit, :destroy, :submit, :save]
before_action :find_launcher, only: [:show, :edit, :destroy, :submit, :save]

SAVE_SCRIPT_KEYS = [
SAVE_LAUNCHER_KEYS = [
:cluster, :auto_accounts, :auto_accounts_exclude, :auto_accounts_fixed,
:auto_cores, :auto_cores_fixed, :auto_cores_min, :auto_cores_max,
:auto_scripts, :auto_scripts_exclude, :auto_scripts_fixed,
Expand All @@ -19,21 +19,21 @@ class LaunchersController < ApplicationController
].freeze

def new
@script = Launcher.new(project_dir: @project.directory)
@launcher = Launcher.new(project_dir: @project.directory)
end

# POST /dashboard/projects/:project_id/launchers
def create
opts = { project_dir: @project.directory }.merge(create_script_params[:launcher])
@script = Launcher.new(opts)
default_script_created = @script.create_default_script
opts = { project_dir: @project.directory }.merge(create_launcher_params[:launcher])
@launcher = Launcher.new(opts)
default_script_created = @launcher.create_default_script

if @script.save
notice_messages = [I18n.t('dashboard.jobs_scripts_created')]
notice_messages << I18n.t('dashboard.jobs_scripts_default_created') if default_script_created
if @launcher.save
notice_messages = [I18n.t('dashboard.jobs_launchers_created')]
notice_messages << I18n.t('dashboard.jobs_launchers_default_created') if default_script_created
redirect_to project_path(params[:project_id]), notice: notice_messages.join(' ')
else
redirect_to project_path(params[:project_id]), alert: @script.errors[:save].last
redirect_to project_path(params[:project_id]), alert: @launcher.errors[:save].last
end
end

Expand All @@ -44,69 +44,69 @@ def edit

# DELETE /projects/:project_id/launchers/:id
def destroy
if @script.destroy
redirect_to project_path(params[:project_id]), notice: I18n.t('dashboard.jobs_scripts_deleted')
if @launcher.destroy
redirect_to project_path(params[:project_id]), notice: I18n.t('dashboard.jobs_launchers_deleted')
else
redirect_to project_path(params[:project_id]), alert: @script.errors[:destroy].last
redirect_to project_path(params[:project_id]), alert: @launcher.errors[:destroy].last
end
end

# POST /projects/:project_id/launchers/:id/save
# save the launcher after editing
def save
@script.update(save_script_params[:launcher])
@launcher.update(save_launcher_params[:launcher])

if @script.save
redirect_to project_path(params[:project_id]), notice: I18n.t('dashboard.jobs_scripts_updated')
if @launcher.save
redirect_to project_path(params[:project_id]), notice: I18n.t('dashboard.jobs_launchers_updated')
else
redirect_to project_path(params[:project_id]), alert: @script.errors[:save].last
redirect_to project_path(params[:project_id]), alert: @launcher.errors[:save].last
end
end

# POST /projects/:project_id/launchers/:id/submit
# submit the job
def submit
opts = submit_script_params[:launcher].to_h.symbolize_keys
opts = submit_launcher_params[:launcher].to_h.symbolize_keys

if (job_id = @script.submit(opts))
redirect_to(project_path(params[:project_id]), notice: I18n.t('dashboard.jobs_scripts_submitted', job_id: job_id))
if (job_id = @launcher.submit(opts))
redirect_to(project_path(params[:project_id]), notice: I18n.t('dashboard.jobs_launchers_submitted', job_id: job_id))
else
redirect_to(project_path(params[:project_id]), alert: @script.errors[:submit].last)
redirect_to(project_path(params[:project_id]), alert: @launcher.errors[:submit].last)
end
end

private

def find_script
@script = Launcher.find(show_script_params[:id], @project.directory)
redirect_to(project_path(@project.id), alert: "Cannot find script #{show_script_params[:id]}") if @script.nil?
def find_launcher
@launcher = Launcher.find(show_launcher_params[:id], @project.directory)
redirect_to(project_path(@project.id), alert: "Cannot find launcher #{show_launcher_params[:id]}") if @launcher.nil?
end

def create_script_params
def create_launcher_params
params.permit({ launcher: [:title] }, :project_id)
end

def show_script_params
def show_launcher_params
params.permit(:id, :project_id)
end

def submit_script_params
keys = @script.smart_attributes.map { |sm| sm.id.to_s }
def submit_launcher_params
keys = @launcher.smart_attributes.map { |sm| sm.id.to_s }
params.permit({ launcher: keys }, :project_id, :id)
end

def save_script_params
def save_launcher_params
auto_env_params = params[:launcher].keys.select do |k|
k.match?('auto_environment_variable')
end
allowlist = SAVE_SCRIPT_KEYS + auto_env_params

allowlist = SAVE_LAUNCHER_KEYS + auto_env_params

params.permit({ launcher: allowlist }, :project_id, :id)
end

def find_project
@project = Project.find(show_script_params[:project_id])
redirect_to(projects_path, alert: "Cannot find project: #{show_script_params[:project_id]}") if @project.nil?
@project = Project.find(show_launcher_params[:project_id])
redirect_to(projects_path, alert: "Cannot find project: #{show_launcher_params[:project_id]}") if @project.nil?
end
end
4 changes: 2 additions & 2 deletions apps/dashboard/app/controllers/projects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ def show
format.json { render json: { message: message }, status: :not_found }
end
else
@scripts = Launcher.all(@project.directory)
@launchers = Launcher.all(@project.directory)
@valid_project = Launcher.clusters?
@valid_scripts = Launcher.scripts?(@project.directory)

alert_messages = []
alert_messages << I18n.t('dashboard.jobs_project_invalid_configuration_clusters') unless @valid_project
alert_messages << I18n.t('dashboard.jobs_project_invalid_configuration_scripts') if @scripts.any? && !@valid_scripts
alert_messages << I18n.t('dashboard.jobs_project_invalid_configuration_scripts') if @launchers.any? && !@valid_scripts
flash.now[:alert] = alert_messages.join(' ') if alert_messages.any?
respond_to do |format|
format.html
Expand Down
34 changes: 17 additions & 17 deletions apps/dashboard/app/models/launcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ def launchers_dir(project_dir)
end

def find(id, project_dir)
script_path = Launcher.script_path(project_dir, id)
file = script_form_file(script_path)
path = Launcher.path(project_dir, id)
file = launcher_form_file(path)
Launcher.from_yaml(file, project_dir)
end

Expand All @@ -37,7 +37,7 @@ def from_yaml(file, project_dir)

new(opts)
rescue StandardError, Errno::ENOENT => e
Rails.logger.warn("Did not find script due to error #{e}")
Rails.logger.warn("Did not find launcher due to error #{e}")
nil
end

Expand Down Expand Up @@ -149,26 +149,26 @@ def save
return false unless valid?(:save)

@created_at = Time.now.to_i if @created_at.nil?
script_path = Launcher.script_path(project_dir, id)
path = Launcher.path(project_dir, id)

script_path.mkpath unless script_path.exist?
File.write(Launcher.script_form_file(script_path), to_yaml)
path.mkpath unless path.exist?
File.write(Launcher.launcher_form_file(path), to_yaml)

true
rescue StandardError => e
errors.add(:save, e.message)
Rails.logger.warn("Cannot save script due to error: #{e.class}:#{e.message}")
Rails.logger.warn("Cannot save launcher due to error: #{e.class}:#{e.message}")
false
end

def destroy
return true unless id
script_path = Launcher.script_path(project_dir, id)
FileUtils.remove_dir(Launcher.script_path(project_dir, id)) if script_path.exist?
path = Launcher.path(project_dir, id)
FileUtils.remove_dir(Launcher.path(project_dir, id)) if path.exist?
true
rescue StandardError => e
errors.add(:destroy, e.message)
Rails.logger.warn("Cannot delete script #{id} due to error: #{e.class}:#{e.message}")
Rails.logger.warn("Cannot delete launcher #{id} due to error: #{e.class}:#{e.message}")
false
end

Expand Down Expand Up @@ -221,20 +221,20 @@ def create_default_script

private

def self.script_path(root_dir, script_id)
unless script_id.to_s.match?(ID_REX)
raise(StandardError, "#{script_id} is invalid. Does not match #{ID_REX.inspect}")
def self.path(root_dir, launcher_id)
unless launcher_id.to_s.match?(ID_REX)
raise(StandardError, "#{launcher_id} is invalid. Does not match #{ID_REX.inspect}")
end

Pathname.new(File.join(Launcher.launchers_dir(root_dir), script_id.to_s))
Pathname.new(File.join(Launcher.launchers_dir(root_dir), launcher_id.to_s))
end

def default_script_path
Pathname(File.join(project_dir, 'hello_world.sh'))
end

def self.script_form_file(script_path)
File.join(script_path, "form.yml")
def self.launcher_form_file(path)
File.join(path, "form.yml")
end

# parameters you got from the controller that affect the attributes, not form.
Expand Down Expand Up @@ -284,7 +284,7 @@ def write_job_options_to_cache(opts)
end

def cache_file_path
Pathname.new(File.join(Launcher.script_path(project_dir, id), "cache.json"))
Pathname.new(File.join(Launcher.path(project_dir, id), "cache.json"))
end

def cache_file_exists?
Expand Down
10 changes: 5 additions & 5 deletions apps/dashboard/app/models/project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -271,11 +271,11 @@
# This creates them _and_ serializes them to yml in the new directory.
def save_new_launchers
dir = Launcher.launchers_dir(template)
Dir.glob("#{dir}/*/form.yml").map do |script_yml|
Launcher.from_yaml(script_yml, project_dataroot)
end.map do |script|
saved_successfully = script.save
errors.add(:save, script.errors.full_messages) unless saved_successfully
Dir.glob("#{dir}/*/form.yml").map do |launcher_yml|

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.

Copilot Autofix AI about 1 month ago

To fix the problem, we need to ensure that the template attribute is properly validated and sanitized before being used to construct file paths. This can be achieved by implementing additional validation rules to restrict the format of the template attribute, ensuring it does not contain any directory traversal sequences or other potentially harmful characters.

  1. Add a custom validation method to sanitize the template attribute.
  2. Update the project_template_invalid method to include the new validation.
  3. Ensure that the template attribute is sanitized before being used in the save_new_launchers method.
Suggested changeset 1
apps/dashboard/app/models/project.rb

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/apps/dashboard/app/models/project.rb b/apps/dashboard/app/models/project.rb
--- a/apps/dashboard/app/models/project.rb
+++ b/apps/dashboard/app/models/project.rb
@@ -272,2 +272,3 @@
   def save_new_launchers
+    sanitize_template
     dir = Launcher.launchers_dir(template)
@@ -310,2 +311,3 @@
 
+    sanitize_template
     template_path = Pathname.new(template)
@@ -314,2 +316,7 @@
   end
+
+  def sanitize_template
+    # Remove any directory traversal sequences and ensure the template is a valid directory name
+    self.template = template.gsub(/(\.\.\/|\/|\\)/, '')
+  end
 end
EOF
@@ -272,2 +272,3 @@
def save_new_launchers
sanitize_template
dir = Launcher.launchers_dir(template)
@@ -310,2 +311,3 @@

sanitize_template
template_path = Pathname.new(template)
@@ -314,2 +316,7 @@
end

def sanitize_template
# Remove any directory traversal sequences and ensure the template is a valid directory name
self.template = template.gsub(/(\.\.\/|\/|\\)/, '')
end
end
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Launcher.from_yaml(launcher_yml, project_dataroot)
end.map do |launcher|
saved_successfully = launcher.save
errors.add(:save, launcher.errors.full_messages) unless saved_successfully

saved_successfully
end.all? do |saved_successfully|
Expand Down
8 changes: 4 additions & 4 deletions apps/dashboard/app/views/launchers/edit.html.erb
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
<h1>Editing <%= @script.title %></h1>
<h1>Editing <%= @launcher.title %></h1>

<%= javascript_include_tag 'launcher_edit', nonce: true %>

<%= link_to 'Back', project_path(params[:project_id]), class: 'btn btn-default my-3', title: 'Return to projects page' %>

<%= bootstrap_form_for(@script, url: save_project_launcher_path, html: { autocomplete: "off" }) do |f| %>
<% @script.smart_attributes.each do |attrib| %>
<%= bootstrap_form_for(@launcher, url: save_project_launcher_path, html: { autocomplete: "off" }) do |f| %>
<% @launcher.smart_attributes.each do |attrib| %>
<%# TODO generate render_format %>
<%= create_editable_widget(f, attrib, format: nil) %>
<% end %>

<%= render partial: 'add_new_field' %>

<div class="d-grid gap-2">
<%= f.submit t('dashboard.save'), class: 'btn btn-primary', id: 'save_script_edit' %>
<%= f.submit t('dashboard.save'), class: 'btn btn-primary', id: 'save_launcher_edit' %>
</div>
<% end %>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
<div class="list-group col-md-4">
<div class="list-group-item mb-3">
<input type="checkbox" id="<%= fixed_id %>" name="<%= fixed_name %>" value="true" <%= fixed ? 'checked' : '' %> data-fixed-toggler="<%= field_id %>">
<label class="form-check-label" for="<%= fixed_id %>"><%= t('dashboard.jobs_scripts_fixed_field') %></label>
<label class="form-check-label" for="<%= fixed_id %>"><%= t('dashboard.jobs_launchers_fixed_field') %></label>
</div>
</div>
2 changes: 1 addition & 1 deletion apps/dashboard/app/views/launchers/new.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

<%= bootstrap_form_for(@script, url: project_launchers_path) do |form| %>
<%= bootstrap_form_for(@launcher, url: project_launchers_path) do |form| %>

<div class="field">
<%= form.text_field :title, label: "Name" %>
Expand Down
6 changes: 3 additions & 3 deletions apps/dashboard/app/views/launchers/show.html.erb
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@

<h1><%= @script.title %></h1>
<h1><%= @launcher.title %></h1>

<%= javascript_include_tag 'script_show', nonce: true %>

<%= link_to 'Back', project_path(params[:project_id]), class: 'btn btn-default my-3', title: 'Return to projects page' %>

<%= bootstrap_form_for(@script, url: submit_project_launcher_path) do |f| %>
<% @script.smart_attributes.each do |attrib| %>
<%= bootstrap_form_for(@launcher, url: submit_project_launcher_path) do |f| %>
<% @launcher.smart_attributes.each do |attrib| %>
<%# TODO generate render_format %>
<%= create_widget(f, attrib, format: nil) %>
<% end %>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
id: "delete_#{launcher.id}",
class: "btn btn-danger launcher-button",
title: delete_title,
data: { confirm: I18n.t('dashboard.jobs_scripts_delete_script_confirmation') },
data: { confirm: I18n.t('dashboard.jobs_launchers_delete_script_confirmation') },
method: :delete) do
%>
<%= I18n.t('dashboard.delete') %>
Expand Down
2 changes: 1 addition & 1 deletion apps/dashboard/app/views/projects/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
</a>

<div id="launcher_list" class="collapse show">
<%- @scripts.each do |launcher| -%>
<%- @launchers.each do |launcher| -%>
<div class="list-group-item list-group-item-action" id="launcher_<%= launcher.id %>">
<div class="row launcher-item">
<div class="col launcher-title">
Expand Down
16 changes: 8 additions & 8 deletions apps/dashboard/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -253,14 +253,14 @@ en:
jobs_project_job_deleted: "Successfully deleted job %{job_id}"
jobs_project_job_not_deleted: "Cannot delete job %{job_id}"

jobs_scripts_created: "Script successfully created!"
jobs_scripts_default_created: "A 'hello_world.sh' was also added to this project."
jobs_scripts_updated: "Script manifest updated!"
jobs_scripts_not_found: "Cannot find script %{script_id}"
jobs_scripts_deleted: "Script successfully deleted!"
jobs_scripts_submitted: "Successfully submitted job %{job_id}."
jobs_scripts_delete_script_confirmation: "Delete all contents of script?"
jobs_scripts_fixed_field: "Fixed Value"
jobs_launchers_created: "Launcher successfully created!"
jobs_launchers_default_created: "A 'hello_world.sh' was also added to this project."
jobs_launchers_updated: "Launcher manifest updated!"
jobs_launchers_not_found: "Cannot find Launcher %{launcher_id}"
jobs_launchers_deleted: "Launcher successfully deleted!"
jobs_launchers_submitted: "Successfully submitted job %{job_id}."
jobs_launchers_delete_script_confirmation: "Delete all contents of launcher?"
jobs_launchers_fixed_field: "Fixed Value"

settings_updated: "Settings updated"
settings_invalid_request: "Invalid settings submitted"
Expand Down
Loading
Loading