Skip to content

Improve install plugin screen #33

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.

Sign up for GitHub

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

Closed
tungleduyxyz wants to merge 1 commit into from
Closed

Improve install plugin screen #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 27 additions & 1 deletion app/controllers/kpm/nodes_info_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,33 @@ def refresh

def install_plugin
trigger_node_plugin_command('INSTALL_PLUGIN')
redirect_to nodes_info_index_path(i: 1)
redirect_to nodes_info_index_path(i: 1, plugin_key: params[:plugin_key], plugin_version: params[:plugin_version])
end

def plugin_status
plugin_key = params[:plugin_key]
plugin_version = params[:plugin_version]

nodes_info = ::KillBillClient::Model::NodesInfo.nodes_info(options_for_klient)
installed = false

nodes_info.each do |node_info|
next if node_info.plugins_info.nil?

node_info.plugins_info.each do |plugin_info|
if plugin_info.plugin_key == plugin_key
# If plugin_version is specified, check for exact version match
# Otherwise, just check if any version of the plugin exists
if plugin_version.blank? || plugin_info.version == plugin_version
installed = true
break
end
end
end
break if installed
end

render json: { installed: installed }
end

def uninstall_plugin
Expand Down
42 changes: 40 additions & 2 deletions app/views/kpm/nodes_info/index.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,20 +112,58 @@
});

<% if @installing %>
var pluginKey = '<%= params[:plugin_key].to_s.gsub("'", "\\'") %>';

Check failure

Code scanning / CodeQL

Incomplete string escaping or encoding High

This does not escape backslash characters in the input.

Copilot Autofix

AI 21 days ago

In general, when embedding untrusted data into JavaScript in an ERB template, avoid hand-rolled gsub escaping. Instead, either:

  • Use Rails-provided helpers like j/escape_javascript, which correctly escape both quotes and backslashes (and other problematic characters) for JavaScript string literals, or
  • Avoid inline JavaScript altogether and pass data via data-* attributes or JSON, then read it from JavaScript.

The minimal, non-breaking fix here is to keep the same overall structure (inline <script> and var pluginKey = '...') but replace the fragile to_s.gsub("'", "\\'") with Rails’ j helper. j escapes backslashes, quotes, newlines, and other characters into safe JavaScript string literal sequences.

Concretely, in app/views/kpm/nodes_info/index.html.erb:

  • Change line 115 from '<%= params[:plugin_key].to_s.gsub("'", "\\'") %>' to use <%= j params[:plugin_key].to_s %> inside the quotes.
  • Likewise, change line 116 to use <%= j params[:plugin_version].to_s %>.

No new imports are required; j/escape_javascript is available in Rails views by default.

Suggested changeset 1
app/views/kpm/nodes_info/index.html.erb

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/views/kpm/nodes_info/index.html.erb b/app/views/kpm/nodes_info/index.html.erb
--- a/app/views/kpm/nodes_info/index.html.erb
+++ b/app/views/kpm/nodes_info/index.html.erb
@@ -112,8 +112,8 @@
     });
 
     <% if @installing %>
-      var pluginKey = '<%= params[:plugin_key].to_s.gsub("'", "\\'") %>';
-      var pluginVersion = '<%= params[:plugin_version].to_s.gsub("'", "\\'") %>';
+      var pluginKey = '<%= j params[:plugin_key].to_s %>';
+      var pluginVersion = '<%= j params[:plugin_version].to_s %>';
       var pollAttempts = 0;
       var maxPollAttempts = 60;
       var pollInterval = 1000;
EOF
@@ -112,8 +112,8 @@
});

<% if @installing %>
var pluginKey = '<%= params[:plugin_key].to_s.gsub("'", "\\'") %>';
var pluginVersion = '<%= params[:plugin_version].to_s.gsub("'", "\\'") %>';
var pluginKey = '<%= j params[:plugin_key].to_s %>';
var pluginVersion = '<%= j params[:plugin_version].to_s %>';
var pollAttempts = 0;
var maxPollAttempts = 60;
var pollInterval = 1000;
Copilot is powered by AI and may make mistakes. Always verify output.
var pluginVersion = '<%= params[:plugin_version].to_s.gsub("'", "\\'") %>';

Check failure

Code scanning / CodeQL

Incomplete string escaping or encoding High

This does not escape backslash characters in the input.

Copilot Autofix

AI 21 days ago

In general, the right fix is to avoid hand-rolling escaping and instead use a proper JavaScript-escaping helper when embedding Ruby strings into JavaScript string literals. In Rails views, the standard is j (alias for escape_javascript), which correctly escapes backslashes, quotes, newlines, etc., for safe inclusion within JavaScript strings.

In this specific file (app/views/kpm/nodes_info/index.html.erb), we should remove the manual gsub("'", "\\'") escaping and replace it with j(...). That preserves existing functionality (embedding the raw parameter value into JavaScript) while making the escaping complete and robust. Concretely, update:

  • Line 115: var pluginKey = '<%= params[:plugin_key].to_s.gsub("'", "\\'") %>';
  • Line 116: var pluginVersion = '<%= params[:plugin_version].to_s.gsub("'", "\\'") %>';

so that each uses j(params[...] .to_s) instead of the gsub call. There is no need for new imports; j is available in Rails views by default. No other behavior needs to change.

Suggested changeset 1
app/views/kpm/nodes_info/index.html.erb

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/views/kpm/nodes_info/index.html.erb b/app/views/kpm/nodes_info/index.html.erb
--- a/app/views/kpm/nodes_info/index.html.erb
+++ b/app/views/kpm/nodes_info/index.html.erb
@@ -112,8 +112,8 @@
     });
 
     <% if @installing %>
-      var pluginKey = '<%= params[:plugin_key].to_s.gsub("'", "\\'") %>';
-      var pluginVersion = '<%= params[:plugin_version].to_s.gsub("'", "\\'") %>';
+      var pluginKey = '<%= j(params[:plugin_key].to_s) %>';
+      var pluginVersion = '<%= j(params[:plugin_version].to_s) %>';
       var pollAttempts = 0;
       var maxPollAttempts = 60;
       var pollInterval = 1000;
EOF
@@ -112,8 +112,8 @@
});

<% if @installing %>
var pluginKey = '<%= params[:plugin_key].to_s.gsub("'", "\\'") %>';
var pluginVersion = '<%= params[:plugin_version].to_s.gsub("'", "\\'") %>';
var pluginKey = '<%= j(params[:plugin_key].to_s) %>';
var pluginVersion = '<%= j(params[:plugin_version].to_s) %>';
var pollAttempts = 0;
var maxPollAttempts = 60;
var pollInterval = 1000;
Copilot is powered by AI and may make mistakes. Always verify output.
var pollAttempts = 0;
var maxPollAttempts = 60;
var pollInterval = 1000;

$('.plugin-link').removeAttr('data-remote').removeAttr('data-method').removeAttr('href');
$('#nodes-table-wrapper').css({ opacity: 0.5 });
$('#install-in-progress').show();
$('#install-in-progress').children().addClass('fa-spin');
setTimeout(fakeOperationComplete, 6000);

function pollPluginStatus() {
pollAttempts++;

if (pollAttempts > maxPollAttempts) {
operationComplete();
return;
}

$.ajax({
url: '<%= plugin_status_path %>',
type: 'GET',
data: { plugin_key: pluginKey, plugin_version: pluginVersion },
dataType: 'json',
success: function(response) {
if (response.installed) {
operationComplete();
} else {
setTimeout(pollPluginStatus, pollInterval);
}
},
error: function() {
setTimeout(pollPluginStatus, pollInterval);
}
});
}

// Start polling after a short initial delay
setTimeout(pollPluginStatus, 2000);
<% end %>

function fakeOperationComplete() {
function operationComplete() {
$('#install-in-progress').hide();
$('.plugin-link').removeClass('loading');
$('.official-plugin').removeClass('loading');
$('.plugin-link').children().removeClass('fa-spin');
$('#nodes-table-wrapper').fadeTo("slow", 0.5);
$('#reload-page').fadeIn("slow");
}

function fakeOperationComplete() {
operationComplete();
}
});
<% end %>
1 change: 1 addition & 0 deletions config/routes.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@
match '/plugin/start' => 'nodes_info#start_plugin', :via => :post, :as => 'plugin_start'
match '/plugin/stop' => 'nodes_info#stop_plugin', :via => :post, :as => 'plugin_stop'
match '/plugin/restart' => 'nodes_info#restart_plugin', :via => :post, :as => 'plugin_restart'
match '/plugin/status' => 'nodes_info#plugin_status', :via => :get, :as => 'plugin_status'
end
end
Loading