diff --git a/.rubocop.yml b/.rubocop.yml index 8aeff3f..809b562 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -14,3 +14,6 @@ Style/Documentation: Metrics/BlockLength: Exclude: - "spec/**/*" + +Security/YAMLLoad: + Enabled: false diff --git a/lib/committer/config/accessor.rb b/lib/committer/config/accessor.rb index e6bac3f..b1120c7 100644 --- a/lib/committer/config/accessor.rb +++ b/lib/committer/config/accessor.rb @@ -28,60 +28,56 @@ def to_h def load_config # Load configs from both locations and merge them - home_config = load_config_from_path(Committer::Config::Constants::CONFIG_FILE) - git_root_config = load_config_from_git_root + + home_config = load_yml_file(read_file_from_home(Committer::Config::Constants::CONFIG_FILE_NAME)) + git_root_config = load_yml_file(read_file_from_git_root(Committer::Config::Constants::CONFIG_FILE_NAME)) raise Committer::ConfigErrors::NotSetup if home_config.empty? && git_root_config.empty? + unless home_config.is_a?(Hash) && git_root_config.is_a?(Hash) + raise Committer::ConfigErrors::FormatError, + 'Config file must be a YAML hash' + end + # Merge configs with git root taking precedence home_config.merge(git_root_config) end - def load_config_from_path(path) - return {} unless File.exist?(path) - - result = YAML.load_file(path) - raise Committer::ConfigErrors::FormatError, 'Config file must be a YAML hash' unless result.is_a?(Hash) - - result + def load_yml_file(contents) + YAML.safe_load(contents, permitted_classes: [Symbol, NilClass, String, Array]) || {} end - def load_file_from_path(path) + def read_file_from_path(path) return '' unless File.exist?(path) File.read(path) end def load_formatting_rules - git_root = Committer::GitHelper.repo_root - unless git_root.empty? - formatting_rules_git_path = File.join(git_root, '.committer', - Committer::Config::Constants::FORMATTING_RULES_FILE_NAME) - end + read_path_prioritized_file(Committer::Config::Constants::FORMATTING_RULES_FILE_NAME) + end - git_path_contents = load_file_from_path(formatting_rules_git_path) if formatting_rules_git_path + def read_file_from_git_root(file_name) + read_file_from_path(File.join(Committer::GitHelper.repo_root, '.committer', file_name)) + end - return git_path_contents unless git_path_contents.empty? + def read_file_from_home(file_name) + read_file_from_path(File.join(Committer::Config::Constants::CONFIG_DIR, file_name)) + end - home_path = File.join(Committer::Config::Constants::CONFIG_DIR, - Committer::Config::Constants::FORMATTING_RULES_FILE_NAME) + def load_default_file(file_name) + read_file_from_path(File.join(Committer::Config::Constants::DEFAULTS_PATH, file_name)) + end - home_path_contents = load_file_from_path(home_path) + def read_path_prioritized_file(file_name) + git_path_contents = read_file_from_git_root(file_name) - return home_path_contents unless home_path_contents.empty? + return git_path_contents unless git_path_contents.empty? - default_path = File.join(Committer::Config::Constants::DEFAULT_PROMPT_PATH, - Committer::Config::Constants::FORMATTING_RULES_FILE_NAME) - load_file_from_path(default_path) - end + home_path_contents = read_file_from_home(file_name) - def load_config_from_git_root - git_root = Committer::GitHelper.repo_root - return {} if git_root.empty? + return home_path_contents unless home_path_contents.empty? - git_config_file = File.join(git_root, '.committer', 'config.yml') - load_config_from_path(git_config_file) - rescue StandardError - {} + load_default_file(file_name) end # Force reload configuration (useful for testing) diff --git a/lib/committer/config/constants.rb b/lib/committer/config/constants.rb index 91235bc..ca638f9 100644 --- a/lib/committer/config/constants.rb +++ b/lib/committer/config/constants.rb @@ -4,10 +4,11 @@ module Committer module Config module Constants CONFIG_DIR = File.join(Dir.home, '.committer') - CONFIG_FILE = File.join(CONFIG_DIR, 'config.yml') + DEFAULTS_PATH = File.join(File.dirname(__FILE__), './defaults') + FORMATTING_RULES_FILE_NAME = 'formatting_rules.txt' - DEFAULT_PROMPT_PATH = File.join(File.dirname(__FILE__), '..') CONFIG_FILE_NAME = 'config.yml' + DEFAULT_CONFIG = { 'api_key' => nil, 'model' => 'claude-3-7-sonnet-20250219', diff --git a/lib/committer/formatting_rules.txt b/lib/committer/config/defaults/formatting_rules.txt similarity index 100% rename from lib/committer/formatting_rules.txt rename to lib/committer/config/defaults/formatting_rules.txt diff --git a/lib/committer/config/writer.rb b/lib/committer/config/writer.rb index 0591d90..da9b4ac 100644 --- a/lib/committer/config/writer.rb +++ b/lib/committer/config/writer.rb @@ -35,7 +35,7 @@ def write_config_file(file_path, contents) end def create_sample_formatting_rules - default_formatting_rules = File.read(File.join(Committer::Config::Constants::DEFAULT_PROMPT_PATH, + default_formatting_rules = File.read(File.join(Committer::Config::Constants::DEFAULTS_PATH, Committer::Config::Constants::FORMATTING_RULES_FILE_NAME)) formatting_rules_file = File.join(@config_dir, "#{Committer::Config::Constants::FORMATTING_RULES_FILE_NAME}.sample") diff --git a/spec/committer/config_spec.rb b/spec/committer/config_spec.rb index 8c9bd95..393dc76 100644 --- a/spec/committer/config_spec.rb +++ b/spec/committer/config_spec.rb @@ -8,11 +8,9 @@ let(:temp_home) { Dir.mktmpdir } let(:config_dir) { File.join(temp_home, '.committer') } let(:config_file) { File.join(config_dir, 'config.yml') } - let(:home_formatting_rules) { File.join(config_dir, 'formatting_rules.txt') } let(:git_root) { Dir.mktmpdir } let(:git_config_dir) { File.join(git_root, '.committer') } let(:git_config_file) { File.join(git_config_dir, 'config.yml') } - let(:git_formatting_rules) { File.join(git_config_dir, 'formatting_rules.txt') } let(:config_instance) { described_class.instance } before do @@ -35,38 +33,38 @@ FileUtils.remove_entry(git_root) if File.directory?(git_root) end - describe 'custom formatting rules' do - before do - FileUtils.mkdir_p(config_dir) - File.write(config_file, { 'api_key' => 'home_key', 'model' => 'home_model', 'scopes' => ['home'] }.to_yaml) - end - - context 'when formatting rules exist in home root' do + describe 'loads file based on config priority paths' do + context 'when file exists in home root' do before do FileUtils.mkdir_p(config_dir) - File.write(home_formatting_rules, '# Custom formatting rules defined in home') + File.write(config_file, { 'api_key' => 'home_key', 'model' => 'home_model', 'scopes' => ['home'] }.to_yaml) end - it 'loads custom formatting rules' do - formatting_rules = config_instance.load_formatting_rules - expect(formatting_rules).to include('# Custom formatting rules defined in home') + it 'loads file from home' do + formatting_rules = config_instance.read_path_prioritized_file('config.yml') + expect(formatting_rules).to include('api_key: home_key') end - context 'when formatting rules exist in git root' do + context 'when file exists in home and in git root' do before do FileUtils.mkdir_p(git_config_dir) - File.write(git_formatting_rules, '# Custom formatting rules defined in git') + File.write(git_config_file, { 'api_key' => 'git_key', 'model' => 'home_model', 'scopes' => ['home'] }.to_yaml) end it 'loads custom formatting rules from git root instead of home' do - formatting_rules = config_instance.load_formatting_rules - expect(formatting_rules).to include('# Custom formatting rules defined in git') + formatting_rules = config_instance.read_path_prioritized_file('config.yml') + expect(formatting_rules).to include('api_key: git_key') end end end + context 'when no custom files exist' do - it 'loads default formatting rules' do - formatting_rules = config_instance.load_formatting_rules + before do + FileUtils.mkdir_p(git_config_dir) + File.write(git_config_file, { 'api_key' => '', 'model' => '', 'scopes' => [] }.to_yaml) + end + it 'loads from default file' do + formatting_rules = config_instance.read_path_prioritized_file('formatting_rules.txt') expect(formatting_rules).to include('# Formatting rules for message') end end @@ -130,15 +128,15 @@ FileUtils.mkdir_p(config_dir) File.write(config_file, 'invalid:yaml:[}') - # Make sure YAML.load_file raises an exception for the home config - allow(YAML).to receive(:load_file).with(config_file).and_raise( + # Make sure YAML.read_path_prioritized_file raises an exception for the home config + allow(YAML).to receive(:safe_load).with(config_file).and_raise( Psych::SyntaxError.new('file', 0, 0, 0, 'invalid syntax', 'problem') ) # But allow normal behavior for other files - allow(YAML).to receive(:load_file).and_call_original + allow(YAML).to receive(:safe_load).and_call_original end - it 'continues execution and uses default config' do + it 'raises format error' do expect { config_instance }.to raise_error(Committer::ConfigErrors::FormatError) end end