Skip to content

Conversation

@pboling
Copy link
Member

@pboling pboling commented Mar 26, 2025

Attempt to keep Rails 5.2 and 6.0 support

@pboling pboling added the enhancement New feature or request label Mar 26, 2025
@pboling pboling self-assigned this Mar 26, 2025
def update
respond_to do |format|
if site.update_attributes(site_params)
if site.update(site_params)

Check failure

Code scanning / CodeQL

Insecure Mass Assignment Critical

This mass assignment operation can assign user-controlled attributes from
this remote flow source
.

Copilot Autofix

AI 8 months ago

To fix the problem, we need to explicitly specify which keys are permitted within the properties hash. This can be done by listing the allowed keys in the permit method. This change should be made in the site_params method in the app/controllers/masq/sites_controller.rb file.

  1. Identify the keys that are allowed within the properties hash.
  2. Update the site_params method to explicitly permit only those keys.
Suggested changeset 1
app/controllers/masq/sites_controller.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/app/controllers/masq/sites_controller.rb b/app/controllers/masq/sites_controller.rb
--- a/app/controllers/masq/sites_controller.rb
+++ b/app/controllers/masq/sites_controller.rb
@@ -56,3 +56,3 @@
     def site_params
-      params.require(:site).permit(:persona_id, :url, properties: {})
+      params.require(:site).permit(:persona_id, :url, properties: [:key1, :key2, :key3])
     end
EOF
@@ -56,3 +56,3 @@
def site_params
params.require(:site).permit(:persona_id, :url, properties: {})
params.require(:site).permit(:persona_id, :url, properties: [:key1, :key2, :key3])
end
Copilot is powered by AI and may make mistakes. Always verify output.

# Encrypts some data with the salt.
def encrypt(password, salt)
Digest::SHA1.hexdigest("--#{salt}--#{password}--")

Check failure

Code scanning / CodeQL

Use of a broken or weak cryptographic hashing algorithm on sensitive data High

Sensitive data (password)
is used in a hashing algorithm (SHA1) that is insecure for password hashing, since it is not a computationally expensive hash function.

Copilot Autofix

AI 8 months ago

To fix the problem, we need to replace the use of the weak SHA-1 hashing algorithm with a stronger, more secure algorithm suitable for password hashing. One of the best options for password hashing is the Argon2 algorithm, which is designed to be computationally expensive and includes a per-password salt by default.

To implement the fix, we will:

  1. Replace the encrypt method to use Argon2 for hashing passwords.
  2. Add the necessary import for the argon2 gem.
  3. Ensure that the authenticate method verifies the password using Argon2.
Suggested changeset 2
app/models/masq/account.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/app/models/masq/account.rb b/app/models/masq/account.rb
--- a/app/models/masq/account.rb
+++ b/app/models/masq/account.rb
@@ -1,2 +1,2 @@
-require "digest/sha1"
+require "argon2"
 
@@ -73,3 +73,3 @@
         if !a.nil? && a.active? && a.enabled
-          if a.authenticated?(password) || (Masq::Engine.config.masq["trust_basic_auth"] && basic_auth_used)
+          if Argon2::Password.verify_password(password, a.encrypted_password) || (Masq::Engine.config.masq["trust_basic_auth"] && basic_auth_used)
             a.last_authenticated_at = Time.now.utc
@@ -82,5 +82,5 @@
 
-      # Encrypts some data with the salt.
+      # Encrypts some data with Argon2.
       def encrypt(password, salt)
-        Digest::SHA1.hexdigest("--#{salt}--#{password}--")
+        Argon2::Password.create(password)
       end
EOF
@@ -1,2 +1,2 @@
require "digest/sha1"
require "argon2"

@@ -73,3 +73,3 @@
if !a.nil? && a.active? && a.enabled
if a.authenticated?(password) || (Masq::Engine.config.masq["trust_basic_auth"] && basic_auth_used)
if Argon2::Password.verify_password(password, a.encrypted_password) || (Masq::Engine.config.masq["trust_basic_auth"] && basic_auth_used)
a.last_authenticated_at = Time.now.utc
@@ -82,5 +82,5 @@

# Encrypts some data with the salt.
# Encrypts some data with Argon2.
def encrypt(password, salt)
Digest::SHA1.hexdigest("--#{salt}--#{password}--")
Argon2::Password.create(password)
end
Gemfile
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Gemfile b/Gemfile
--- a/Gemfile
+++ b/Gemfile
@@ -50 +50,3 @@
 gem "rails", "~> 8.0", ">= 8.0.2"
+
+gem "argon2", "2.3.2"
EOF
@@ -50 +50,3 @@
gem "rails", "~> 8.0", ">= 8.0.2"

gem "argon2", "2.3.2"
This fix introduces these dependencies
Package Version Security advisories
argon2 (rubygems) 2.3.2 None
Copilot is powered by AI and may make mistakes. Always verify output.
@codecov
Copy link

codecov bot commented Mar 26, 2025

Codecov Report

Attention: Patch coverage is 85.41667% with 42 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@58dc914). Learn more about missing BASE report.

Files with missing lines Patch % Lines
app/controllers/masq/accounts_controller.rb 62.96% 10 Missing ⚠️
...masq/active_record_openid_store/openid_ar_store.rb 33.33% 8 Missing ⚠️
lib/masq.rb 57.14% 6 Missing ⚠️
app/controllers/masq/server_controller.rb 76.19% 5 Missing ⚠️
app/controllers/masq/personas_controller.rb 77.77% 2 Missing ⚠️
app/controllers/masq/sessions_controller.rb 87.50% 2 Missing ⚠️
app/models/masq/account.rb 97.14% 2 Missing ⚠️
app/models/masq/open_id_request.rb 83.33% 2 Missing ⚠️
app/controllers/masq/base_controller.rb 90.00% 1 Missing ⚠️
app/controllers/masq/sites_controller.rb 85.71% 1 Missing ⚠️
... and 3 more
Additional details and impacted files
@@           Coverage Diff           @@
##             main       #5   +/-   ##
=======================================
  Coverage        ?   88.33%           
=======================================
  Files           ?       28           
  Lines           ?      917           
  Branches        ?      290           
=======================================
  Hits            ?      810           
  Misses          ?      107           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pboling pboling changed the title Combustion.2 ✨ Combustion & Appraisals Mar 26, 2025
@pboling pboling merged commit 8d34f06 into main Mar 26, 2025
21 of 22 checks passed
@pboling pboling deleted the combustion.2 branch March 26, 2025 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants