Disallow direct env access

README.md

@@ -4,6 +4,7 @@
 
 1. [Neeto/UnsafeTableDeletion](https://rubocop-neeto.neetodeployapp.com/docs/RuboCop/Cop/Neeto/UnsafeTableDeletion)
 2. [Neeto/UnsafeColumnDeletion](https://rubocop-neeto.neetodeployapp.com/docs/RuboCop/Cop/Neeto/UnsafeColumnDeletion)
+3. [Neeto/DirectEnvAccess](https://rubocop-neeto.neetodeployapp.com/docs/RuboCop/Cop/Neeto/DirectEnvAccess)
 
 ## Installation
 

config/default.yml

@@ -23,3 +23,17 @@ Neeto/UnsafeColumnDeletion:
   VersionAdded: '0.1'
   Include:
     - db/**/*.rb
+
+Neeto/DirectEnvAccess:
+  Description: >-
+    Rails had `secrets.yml` which provided a single source of truth for all
+    environment variables and their fallback values. Rails deprecated this in
+    favor of encrypted credentials, so we created Secvault to maintain
+    centralized configuration. Direct usage of `ENV` bypasses this system,
+    making it harder to track what environment variables are being used and
+    their defaults. Use `Secvault.secrets` instead.
+  Enabled: true
+  Severity: refactor
+  VersionAdded: '0.1'
+  Include:
+    - app/**/*.rb

lib/rubocop/cop/neeto/direct_env_access.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Neeto
+      # Rails had `secrets.yml` which provided a single source of truth for all
+      # environment variables and their fallback values. Rails deprecated this in
+      # favor of encrypted credentials, so we created Secvault
+      # (https://github.com/neetozone/secvault) to maintain centralized configuration.
+      # Direct usage of `ENV` bypasses this system, making it harder to track what
+      # environment variables are being used and their defaults. This cop enforces
+      # that all environment variable access goes through `Secvault.secrets`.
+      #
+      # @example DirectEnvAccess: true (default)
+      #   # Enforces the usage of `Secvault.secrets` over direct `ENV` access.
+      #
+      #   # bad
+      #   api_key = ENV['STRIPE_API_KEY']
+      #
+      #   # bad
+      #   default_timezone = ENV['DEFAULT_TIMEZONE'] || 'UTC'
+      #
+      #   # good
+      #   api_key = Secvault.secrets.stripe_api_key
+      #
+      #   # good
+      #   default_timezone = Secvault.secrets.default_timezone
+      #
+      #   # good (ENV access is permitted in directories other than the app directory)
+      #   config.log_level = ENV.fetch('LOG_LEVEL', 'info')
+      #
+      class DirectEnvAccess < Base
+        MSG = "Do not use ENV directly. " \
+        "Use Secvault.secrets to maintain a single source of truth for configuration."
+
+        def_node_matcher :env_access?, <<~PATTERN
+          (send (const {nil? cbase} :ENV) _ ...)
+        PATTERN
+
+        def on_send(node)
+          return unless env_access?(node)
+
+          add_offense(node)
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/neeto_cops.rb

@@ -2,3 +2,4 @@
 
 require_relative "neeto/unsafe_table_deletion"
 require_relative "neeto/unsafe_column_deletion"
+require_relative "neeto/direct_env_access"

spec/rubocop/cop/neeto/direct_env_access_spec.rb

@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Neeto::DirectEnvAccess, :config do
+  let(:config) { RuboCop::Config.new }
+
+  it "registers an offense when ENV is accessed with bracket notation" do
+    snippet = <<~RUBY
+      api_key = ENV['STRIPE_API_KEY']
+                ^^^^^^^^^^^^^^^^^^^^^ #{offense}
+    RUBY
+    expect_offense(snippet)
+  end
+
+  it "registers an offense when ENV.fetch is used" do
+    snippet = <<~RUBY
+      default_timezone = ENV.fetch('DEFAULT_TIMEZONE', 'UTC')
+                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{offense}
+    RUBY
+    expect_offense(snippet)
+  end
+
+  it "registers an offense when ENV.fetch is used without a default" do
+    snippet = <<~RUBY
+      api_key = ENV.fetch('STRIPE_API_KEY')
+                ^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{offense}
+    RUBY
+    expect_offense(snippet)
+  end
+
+  it "registers multiple offenses when ENV is accessed multiple times" do
+    snippet = <<~RUBY
+      api_key = ENV['STRIPE_API_KEY']
+                ^^^^^^^^^^^^^^^^^^^^^ #{offense}
+      timeout = ENV.fetch('REQUEST_TIMEOUT', '30')
+                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{offense}
+    RUBY
+    expect_offense(snippet)
+  end
+
+  it "registers an offense when ENV is accessed with :: prefix" do
+    snippet = <<~RUBY
+      api_key = ::ENV['STRIPE_API_KEY']
+                ^^^^^^^^^^^^^^^^^^^^^^^ #{offense}
+    RUBY
+    expect_offense(snippet)
+  end
+
+  it "does not register an offense when Secvault.secrets is used" do
+    snippet = <<~RUBY
+      api_key = Secvault.secrets.stripe_api_key
+    RUBY
+    expect_no_offenses(snippet)
+  end
+
+  it "does not register an offense for non-ENV constants" do
+    snippet = <<~RUBY
+      value = SOME_CONSTANT['key']
+    RUBY
+    expect_no_offenses(snippet)
+  end
+
+  private
+
+    def offense
+      "Neeto/DirectEnvAccess: #{RuboCop::Cop::Neeto::DirectEnvAccess::MSG}"
+    end
+end