Add new Rails/EnvironmentVariableAccess cop

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## master (unreleased)
 
+### New features
+
+* Add new `Rails/EnvironmentVariableAccess` cop. ([@drenmi][])
+
 ### Bug fixes
 
 * [#421](https://github.com/rubocop/rubocop-rails/issues/421): Fix incorrect auto-correct for `Rails/LinkToBlank` when using `target: '_blank'` with hash brackets for the option. ([@koic][])
@@ -339,3 +343,4 @@
 [@cilim]: https://github.com/cilim
 [@flanger001]: https://github.com/flanger001
 [@ohbarye]: https://github.com/ohbarye
+[@drenmi]: https://github.com/drenmi

config/default.yml

@@ -237,6 +237,18 @@ Rails/EnvironmentComparison:
   Enabled: true
   VersionAdded: '0.52'
 
+Rails/EnvironmentVariableAccess:
+  Description: 'Do not access `ENV` directly after initialization.'
+  Enabled: pending
+  VersionAdded: '2.10'
+  Include:
+    - app/**/*.rb
+    - lib/**/*.rb
+  Exclude:
+    - lib/**/*.rake
+  AllowReads: false
+  AllowWrites: false
+
 Rails/Exit:
   Description: >-
                   Favor `fail`, `break`, `return`, etc. over `exit` in

docs/modules/ROOT/pages/cops.adoc

@@ -42,6 +42,7 @@ based on the https://rails.rubystyle.guide/[Rails Style Guide].
 * xref:cops_rails.adoc#railsenumhash[Rails/EnumHash]
 * xref:cops_rails.adoc#railsenumuniqueness[Rails/EnumUniqueness]
 * xref:cops_rails.adoc#railsenvironmentcomparison[Rails/EnvironmentComparison]
+* xref:cops_rails.adoc#railsenvironmentvariableaccess[Rails/EnvironmentVariableAccess]
 * xref:cops_rails.adoc#railsexit[Rails/Exit]
 * xref:cops_rails.adoc#railsfilepath[Rails/FilePath]
 * xref:cops_rails.adoc#railsfindby[Rails/FindBy]

docs/modules/ROOT/pages/cops_rails.adoc

@@ -1282,6 +1282,63 @@ Rails.env == :test
 Rails.env.production?
 ----
 
+== Rails/EnvironmentVariableAccess
+
+|===
+| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
+
+| Pending
+| Yes
+| No
+| 2.10
+| -
+|===
+
+This cop looks for direct access to environment variables through the
+`ENV` variable within the application code. This can lead to runtime
+errors due to misconfiguration that could have been discovered at boot
+time if the environment variables were loaded as part of initialization
+and copied into the application's configuration or secrets. The cop can
+be configured to allow either reads or writes if required.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+ENV["FOO"]
+ENV.fetch("FOO")
+ENV["FOO"] = "bar"
+
+# good
+Rails.application.config.foo
+Rails.application.config.foo = "bar"
+Rails.application.config.x.foo.bar
+Rails.application.secrets.foo
+----
+
+=== Configurable attributes
+
+|===
+| Name | Default value | Configurable values
+
+| Include
+| `app/**/*.rb`, `lib/**/*.rb`
+| Array
+
+| Exclude
+| `lib/**/*.rake`
+| Array
+
+| AllowReads
+| `false`
+| Boolean
+
+| AllowWrites
+| `false`
+| Boolean
+|===
+
 == Rails/Exit
 
 |===

lib/rubocop/cop/rails/environment_variable_access.rb

@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # This cop looks for direct access to environment variables through the
+      # `ENV` variable within the application code. This can lead to runtime
+      # errors due to misconfiguration that could have been discovered at boot
+      # time if the environment variables were loaded as part of initialization
+      # and copied into the application's configuration or secrets. The cop can
+      # be configured to allow either reads or writes if required.
+      #
+      # @example
+      #   # bad
+      #   ENV["FOO"]
+      #   ENV.fetch("FOO")
+      #   ENV["FOO"] = "bar"
+      #
+      #   # good
+      #   Rails.application.config.foo
+      #   Rails.application.config.foo = "bar"
+      #   Rails.application.config.x.foo.bar
+      #   Rails.application.secrets.foo
+      class EnvironmentVariableAccess < Base
+        READ_MSG = 'Do not read from `ENV` directly post initialization.'
+        WRITE_MSG = 'Do not write to `ENV` directly post initialization.'
+
+        def on_const(node)
+          add_offense(node, message: READ_MSG) if env_read?(node) && !allow_reads?
+          add_offense(node, message: WRITE_MSG) if env_write?(node) && !allow_writes?
+        end
+
+        def_node_search :env_read?, <<~PATTERN
+          ^(send (const {cbase nil?} :ENV) !:[]= ...)
+        PATTERN
+
+        def_node_search :env_write?, <<~PATTERN
+          {^(indexasgn (const {cbase nil?} :ENV) ...)
+           ^(send (const {cbase nil?} :ENV) :[]= ...)}
+        PATTERN
+
+        private
+
+        def allow_reads?
+          cop_config['AllowReads'] == true
+        end
+
+        def allow_writes?
+          cop_config['AllowWrites'] == true
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -31,6 +31,7 @@
 require_relative 'rails/enum_hash'
 require_relative 'rails/enum_uniqueness'
 require_relative 'rails/environment_comparison'
+require_relative 'rails/environment_variable_access'
 require_relative 'rails/exit'
 require_relative 'rails/file_path'
 require_relative 'rails/find_by'

spec/rubocop/cop/rails/environment_variable_access_spec.rb

@@ -0,0 +1,84 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::EnvironmentVariableAccess, :config do
+  context 'when not allowing reads' do
+    it 'registers an offense when indexing into `ENV`' do
+      expect_offense(<<~RUBY)
+        ENV["foo"]
+        ^^^ Do not read from `ENV` directly post initialization.
+      RUBY
+    end
+
+    it 'registers an offense when calling methods on `ENV`' do
+      expect_offense(<<~RUBY)
+        ENV.fetch("FOO")
+        ^^^ Do not read from `ENV` directly post initialization.
+      RUBY
+    end
+
+    it 'registers an offense when calling methods on dereferenced `ENV`' do
+      expect_offense(<<~RUBY)
+        ::ENV.fetch("FOO")
+        ^^^^^ Do not read from `ENV` directly post initialization.
+      RUBY
+    end
+
+    it 'does not register an offense on namespaced `ENV` access' do
+      expect_no_offenses(<<~RUBY)
+        Foo::ENV.fetch("BAR")
+      RUBY
+    end
+  end
+
+  context 'when allowing reads' do
+    let(:cop_config) do
+      { 'AllowReads' => true }
+    end
+
+    it 'does not register an offense when indexing into `ENV`' do
+      expect_no_offenses(<<~RUBY)
+        ENV["foo"]
+      RUBY
+    end
+
+    it 'does not register an offense when calling methods on `ENV`' do
+      expect_no_offenses(<<~RUBY)
+        ENV.fetch("FOO")
+      RUBY
+    end
+
+    it 'does not register an offense when calling methods on dereferenced `ENV`' do
+      expect_no_offenses(<<~RUBY)
+        ::ENV.fetch("FOO")
+      RUBY
+    end
+  end
+
+  context 'when not allowing writes' do
+    it 'registers an offense when writing to an `ENV` key' do
+      expect_offense(<<~RUBY)
+        ENV["foo"] = "bar"
+        ^^^ Do not write to `ENV` directly post initialization.
+      RUBY
+    end
+
+    it 'registers an offense when writing to a dereferenced `ENV` key' do
+      expect_offense(<<~RUBY)
+        ::ENV["foo"] = "bar"
+        ^^^^^ Do not write to `ENV` directly post initialization.
+      RUBY
+    end
+  end
+
+  context 'when allowing writes' do
+    let(:cop_config) do
+      { 'AllowWrites' => true }
+    end
+
+    it 'does not register an offense when writing to an `ENV` key' do
+      expect_no_offenses(<<~RUBY)
+        ENV["foo"] = "bar"
+      RUBY
+    end
+  end
+end