Skip to content

Improve install plugin screen#33

Closed
tungleduyxyz wants to merge 1 commit intomasterfrom
kaui_3.04
Closed

Improve install plugin screen#33
tungleduyxyz wants to merge 1 commit intomasterfrom
kaui_3.04

Conversation

@tungleduyxyz
Copy link
Contributor

Issues:

});

<% 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 20 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.

<% if @installing %>
var pluginKey = '<%= params[:plugin_key].to_s.gsub("'", "\\'") %>';
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 20 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.
@tungleduyxyz tungleduyxyz marked this pull request as draft March 2, 2026 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant