diff --git a/README.md b/README.md index 39abe65..4cd05d1 100644 --- a/README.md +++ b/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 diff --git a/config/default.yml b/config/default.yml index bb6c7d7..09773af 100644 --- a/config/default.yml +++ b/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 diff --git a/lib/rubocop/cop/neeto/direct_env_access.rb b/lib/rubocop/cop/neeto/direct_env_access.rb new file mode 100644 index 0000000..549abf5 --- /dev/null +++ b/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 + (const {nil? cbase} :ENV) + PATTERN + + def on_const(node) + return unless env_access?(node) + + add_offense(node) + end + end + end + end +end diff --git a/lib/rubocop/cop/neeto_cops.rb b/lib/rubocop/cop/neeto_cops.rb index 38ef8e9..874ccdd 100644 --- a/lib/rubocop/cop/neeto_cops.rb +++ b/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" diff --git a/rubocop-neeto-0.1.10.gem b/rubocop-neeto-0.1.10.gem new file mode 100644 index 0000000..42a3a71 Binary files /dev/null and b/rubocop-neeto-0.1.10.gem differ diff --git a/spec/rubocop/cop/neeto/direct_env_access_spec.rb b/spec/rubocop/cop/neeto/direct_env_access_spec.rb new file mode 100644 index 0000000..9ac7b5f --- /dev/null +++ b/spec/rubocop/cop/neeto/direct_env_access_spec.rb @@ -0,0 +1,83 @@ +# 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 "registers an offense when ENV is assigned to a variable" do + snippet = <<~RUBY + env_vars = ENV + ^^^ #{offense} + api_key = env_vars['STRIPE_API_KEY'] + RUBY + expect_offense(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 + + 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