From 0a8ad1bfc5c92ff4992c95a6103e9b7c36fd7723 Mon Sep 17 00:00:00 2001
From: Koichi ITO <koic.ito@gmail.com>
Date: Thu, 17 Feb 2022 00:32:55 +0900
Subject: [PATCH] Fix a false positive for `Security/YamlLoad`

This PR fixes a false positive for `Security/YamlLoad`
when using Ruby 3.1+ (Psych 4).
And it also makes the examples a bit more specific.

Ruby 3.1+ (Psych 4) uses `Psych.load` as `Psych.safe_load` by default.
https://github.com/ruby/psych/pull/487

We may consider removing this cop in the future, but not yet.
---
 ...a_false_positive_for_security_yaml_load.md |  1 +
 lib/rubocop/cop/security/yaml_load.rb         | 12 ++++++++---
 lib/rubocop/cop/variable_force.rb             |  2 +-
 spec/rubocop/cop/security/yaml_load_spec.rb   | 20 +++++++++++++++++--
 4 files changed, 29 insertions(+), 6 deletions(-)
 create mode 100644 changelog/fix_a_false_positive_for_security_yaml_load.md

diff --git a/changelog/fix_a_false_positive_for_security_yaml_load.md b/changelog/fix_a_false_positive_for_security_yaml_load.md
new file mode 100644
index 00000000000..54e4460d456
--- /dev/null
+++ b/changelog/fix_a_false_positive_for_security_yaml_load.md
@@ -0,0 +1 @@
+* [#10424](https://github.com/rubocop/rubocop/pull/10424): Fix a false positive for `Security/YamlLoad` when using Ruby 3.1+ (Psych 4). ([@koic][])
diff --git a/lib/rubocop/cop/security/yaml_load.rb b/lib/rubocop/cop/security/yaml_load.rb
index 16dba0650bf..39881be3410 100644
--- a/lib/rubocop/cop/security/yaml_load.rb
+++ b/lib/rubocop/cop/security/yaml_load.rb
@@ -7,17 +7,21 @@ module Security
       # potential security issues leading to remote code execution when
       # loading from an untrusted source.
       #
+      # NOTE: Ruby 3.1+ (Psych 4) uses `Psych.load` as `Psych.safe_load` by default.
+      #
       # @safety
       #   The behaviour of the code might change depending on what was
       #   in the YAML payload, since `YAML.safe_load` is more restrictive.
       #
       # @example
       #   # bad
-      #   YAML.load("--- foo")
+      #   YAML.load("--- !ruby/object:Foo {}") # Psych 3 is unsafe by default
       #
       #   # good
-      #   YAML.safe_load("--- foo")
-      #   YAML.dump("foo")
+      #   YAML.safe_load("--- !ruby/object:Foo {}", [Foo])                    # Ruby 2.5  (Psych 3)
+      #   YAML.safe_load("--- !ruby/object:Foo {}", permitted_classes: [Foo]) # Ruby 3.0- (Psych 3)
+      #   YAML.load("--- !ruby/object:Foo {}", permitted_classes: [Foo])      # Ruby 3.1+ (Psych 4)
+      #   YAML.dump(foo)
       #
       class YAMLLoad < Base
         extend AutoCorrector
@@ -31,6 +35,8 @@ class YAMLLoad < Base
         PATTERN
 
         def on_send(node)
+          return if target_ruby_version >= 3.1
+
           yaml_load(node) do
             add_offense(node.loc.selector) do |corrector|
               corrector.replace(node.loc.selector, 'safe_load')
diff --git a/lib/rubocop/cop/variable_force.rb b/lib/rubocop/cop/variable_force.rb
index 581c8da492e..e529611b6fa 100644
--- a/lib/rubocop/cop/variable_force.rb
+++ b/lib/rubocop/cop/variable_force.rb
@@ -190,7 +190,7 @@ def regexp_captured_names(node)
           child.children.first
         end.join || ''
 
-        regexp = Regexp.new(regexp_string)
+        regexp = Regexp.new(regexp_string) # FIXME: Need to be handle `Regexp.new(/\x82/n)`.
 
         regexp.named_captures.keys
       end
diff --git a/spec/rubocop/cop/security/yaml_load_spec.rb b/spec/rubocop/cop/security/yaml_load_spec.rb
index 1c3e358d8c9..ee5d3f088c8 100644
--- a/spec/rubocop/cop/security/yaml_load_spec.rb
+++ b/spec/rubocop/cop/security/yaml_load_spec.rb
@@ -15,12 +15,12 @@
 
   it 'registers an offense and corrects load with a literal string' do
     expect_offense(<<~RUBY)
-      YAML.load("--- foo")
+      YAML.load("--- !ruby/object:Foo {}")
            ^^^^ Prefer using `YAML.safe_load` over `YAML.load`.
     RUBY
 
     expect_correction(<<~RUBY)
-      YAML.safe_load("--- foo")
+      YAML.safe_load("--- !ruby/object:Foo {}")
     RUBY
   end
 
@@ -34,4 +34,20 @@
       ::YAML.safe_load("--- foo")
     RUBY
   end
+
+  # Ruby 3.1+ (Psych 4) uses `Psych.load` as `Psych.safe_load` by default.
+  # https://github.com/ruby/psych/pull/487
+  context 'Ruby >= 3.1', :ruby31 do
+    it 'does not register an offense and corrects load with a literal string' do
+      expect_no_offenses(<<~RUBY)
+        YAML.load("--- !ruby/object:Foo {}", permitted_classes: [Foo])
+      RUBY
+    end
+
+    it 'does not register an offense and corrects a fully qualified `::YAML.load`' do
+      expect_no_offenses(<<~RUBY)
+        ::YAML.load("--- !ruby/object:Foo {}", permitted_classes: [Foo])
+      RUBY
+    end
+  end
 end