From 6124ec3158684b2eae226c1814f2cbeac7fbc766 Mon Sep 17 00:00:00 2001 From: Steven Pritchard Date: Thu, 22 Jan 2026 09:58:16 -0600 Subject: [PATCH 1/3] Validate parameter names --- lib/scelint.rb | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/scelint.rb b/lib/scelint.rb index aa90f98..4ed71e9 100644 --- a/lib/scelint.rb +++ b/lib/scelint.rb @@ -396,7 +396,29 @@ def check_type(file, check, file_data) # @param check [String] The name of the check # @param parameter [String] The parameter to validate def check_parameter(file, check, parameter) - errors << "#{file} (check '#{check}'): invalid parameter '#{parameter}'" unless parameter.is_a?(String) && !parameter.empty? + # Regular expression to match valid Puppet class parameter names + valid_parameter = %r{\A([a-z][a-z0-9_]*::)+[a-z][a-z0-9_]*\z} + # From https://www.puppet.com/docs/puppet/7/lang_reserved.html + reserved_words = (%w[and application attr case component consumes default define elsif environment false function if import in inherits node or private produces regexp site true type undef unit unless] + + %w[main settings init] + + %w[any array binary boolean catalogentry class collection callable data default deferred enum float hash integer notundef numeric optional pattern resource regexp runtime scalar semver semVerRange sensitive string struct timespan timestamp tuple type undef variant] + + %w[facts trusted server_facts title name]).freeze + + unless parameter.is_a?(String) && !parameter.empty? + errors << "#{file} (check '#{check}'): invalid parameter '#{parameter}'" + return + end + + unless parameter.match?(valid_parameter) + errors << "#{file} (check '#{check}'): invalid parameter name '#{parameter}'" + return + end + + parameter.split('::').each do |part| + if reserved_words.include?(part) + errors << "#{file} (check '#{check}'): parameter name '#{parameter}' contains reserved word '#{part}'" + end + end end # Check remediation From 230342f55692b2a9d1dd8f8c1429258cc50b51d7 Mon Sep 17 00:00:00 2001 From: Steven Pritchard Date: Thu, 22 Jan 2026 14:17:06 -0600 Subject: [PATCH 2/3] Fix syntax and formatting --- lib/scelint.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/scelint.rb b/lib/scelint.rb index 4ed71e9..e0d5ac0 100644 --- a/lib/scelint.rb +++ b/lib/scelint.rb @@ -399,10 +399,14 @@ def check_parameter(file, check, parameter) # Regular expression to match valid Puppet class parameter names valid_parameter = %r{\A([a-z][a-z0-9_]*::)+[a-z][a-z0-9_]*\z} # From https://www.puppet.com/docs/puppet/7/lang_reserved.html - reserved_words = (%w[and application attr case component consumes default define elsif environment false function if import in inherits node or private produces regexp site true type undef unit unless] - + %w[main settings init] - + %w[any array binary boolean catalogentry class collection callable data default deferred enum float hash integer notundef numeric optional pattern resource regexp runtime scalar semver semVerRange sensitive string struct timespan timestamp tuple type undef variant] - + %w[facts trusted server_facts title name]).freeze + reserved_words = ['and', 'application', 'attr', 'case', 'component', 'consumes', 'default', 'define', 'elsif', + 'environment', 'false', 'function', 'if', 'import', 'in', 'inherits', 'node', + 'or', 'private', 'produces', 'regexp', 'site', 'true', 'type', 'undef', 'unit', 'unless', + 'main', 'settings', 'init', 'any', 'array', 'binary', 'boolean', 'catalogentry', 'class', + 'collection', 'callable', 'data', 'default', 'deferred', 'enum', 'float', 'hash', 'integer', + 'notundef', 'numeric', 'optional', 'pattern', 'resource', 'regexp', 'runtime', 'scalar', + 'semver', 'semVerRange', 'sensitive', 'string', 'struct', 'timespan', 'timestamp', 'tuple', + 'type', 'undef', 'variant', 'facts', 'trusted', 'server_facts', 'title', 'name'].freeze unless parameter.is_a?(String) && !parameter.empty? errors << "#{file} (check '#{check}'): invalid parameter '#{parameter}'" From 3c9a595eff7e874ac575811289e8594ccbf5e919 Mon Sep 17 00:00:00 2001 From: Steven Pritchard Date: Thu, 5 Mar 2026 21:12:16 +0000 Subject: [PATCH 3/3] Add tests --- .../unit/scelint/lint_check_parameter_spec.rb | 202 ++++++++++++++++++ 1 file changed, 202 insertions(+) create mode 100644 spec/unit/scelint/lint_check_parameter_spec.rb diff --git a/spec/unit/scelint/lint_check_parameter_spec.rb b/spec/unit/scelint/lint_check_parameter_spec.rb new file mode 100644 index 0000000..ad14c40 --- /dev/null +++ b/spec/unit/scelint/lint_check_parameter_spec.rb @@ -0,0 +1,202 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'tmpdir' +require 'fileutils' + +RSpec.describe Scelint::Lint do + subject(:lint) { described_class.new([tmpdir]) } + + let(:tmpdir) { Dir.mktmpdir } + + after(:each) { FileUtils.rm_rf(tmpdir) } + + describe '#check_parameter' do + before(:each) { lint.errors.clear } + + context 'with valid parameters' do + it 'accepts a simple two-part parameter' do + lint.check_parameter('file', 'check', 'module::param') + expect(lint.errors).to be_empty + end + + it 'accepts a three-part parameter' do + lint.check_parameter('file', 'check', 'module::subclass::param') + expect(lint.errors).to be_empty + end + + it 'accepts parameters with digits and underscores' do + lint.check_parameter('file', 'check', 'module2::my_param_1') + expect(lint.errors).to be_empty + end + + it 'accepts a parameter with multiple underscores' do + lint.check_parameter('file', 'check', 'my_module::my_class::my_param') + expect(lint.errors).to be_empty + end + end + + context 'when parameter is not a non-empty string' do + it 'errors on nil' do + lint.check_parameter('file', 'check', nil) + expect(lint.errors).to include("file (check 'check'): invalid parameter ''") + end + + it 'errors on empty string' do + lint.check_parameter('file', 'check', '') + expect(lint.errors).to include("file (check 'check'): invalid parameter ''") + end + + it 'errors on an integer' do + lint.check_parameter('file', 'check', 42) + expect(lint.errors).to include("file (check 'check'): invalid parameter '42'") + end + + it 'does not produce an invalid parameter name error (returns early)' do + lint.check_parameter('file', 'check', nil) + expect(lint.errors).not_to include(match(%r{invalid parameter name})) + end + end + + context 'when parameter format is invalid' do + it 'errors when there is no namespace separator' do + lint.check_parameter('file', 'check', 'param_only') + expect(lint.errors).to include("file (check 'check'): invalid parameter name 'param_only'") + end + + it 'errors when the parameter starts with an uppercase letter' do + lint.check_parameter('file', 'check', 'Module::param') + expect(lint.errors).to include("file (check 'check'): invalid parameter name 'Module::param'") + end + + it 'errors when a segment starts with an uppercase letter' do + lint.check_parameter('file', 'check', 'module::MyParam') + expect(lint.errors).to include("file (check 'check'): invalid parameter name 'module::MyParam'") + end + + it 'errors when the parameter starts with an underscore' do + lint.check_parameter('file', 'check', '_module::param') + expect(lint.errors).to include("file (check 'check'): invalid parameter name '_module::param'") + end + + it 'errors when the parameter contains a hyphen' do + lint.check_parameter('file', 'check', 'module::my-param') + expect(lint.errors).to include("file (check 'check'): invalid parameter name 'module::my-param'") + end + + it 'errors when the parameter has a trailing ::' do + lint.check_parameter('file', 'check', 'module::') + expect(lint.errors).to include("file (check 'check'): invalid parameter name 'module::'") + end + + it 'errors when the parameter has a leading ::' do + lint.check_parameter('file', 'check', '::module::param') + expect(lint.errors).to include("file (check 'check'): invalid parameter name '::module::param'") + end + + it 'errors when the parameter uses a single colon instead of ::' do + lint.check_parameter('file', 'check', 'classname:parameter') + expect(lint.errors).to include("file (check 'check'): invalid parameter name 'classname:parameter'") + end + + it 'does not produce a reserved word error (returns early)' do + lint.check_parameter('file', 'check', 'param_only') + expect(lint.errors).not_to include(match(%r{reserved word})) + end + end + + context 'when parameter contains a reserved word' do + it 'errors when the last segment is a reserved word' do + lint.check_parameter('file', 'check', 'module::type') + expect(lint.errors).to include("file (check 'check'): parameter name 'module::type' contains reserved word 'type'") + end + + it 'errors when the first segment is a reserved word' do + lint.check_parameter('file', 'check', 'class::param') + expect(lint.errors).to include("file (check 'check'): parameter name 'class::param' contains reserved word 'class'") + end + + it 'errors when a middle segment is a reserved word' do + lint.check_parameter('file', 'check', 'module::if::param') + expect(lint.errors).to include("file (check 'check'): parameter name 'module::if::param' contains reserved word 'if'") + end + + it 'produces one error per reserved word segment' do + lint.check_parameter('file', 'check', 'class::type') + expect(lint.errors).to include("file (check 'check'): parameter name 'class::type' contains reserved word 'class'") + expect(lint.errors).to include("file (check 'check'): parameter name 'class::type' contains reserved word 'type'") + end + + it 'does not error on a parameter that merely contains a reserved word as a substring' do + lint.check_parameter('file', 'check', 'module::typewriter') + expect(lint.errors).to be_empty + end + end + end + + describe 'parameter validation via full lint' do + def write_checks(dir, checks_yaml) + profile_dir = File.join(dir, 'SIMP', 'compliance_profiles') + FileUtils.mkdir_p(profile_dir) + File.write(File.join(profile_dir, 'checks.yaml'), checks_yaml) + end + + context 'with an unnamespaced parameter' do + before(:each) do + write_checks(tmpdir, <<~YAML) + --- + version: '2.0.0' + checks: + test_check: + type: puppet-class-parameter + settings: + parameter: bad_param + value: foo + YAML + end + + it 'produces an invalid parameter name error' do + expect(lint.errors).to include(match(%r{invalid parameter name 'bad_param'})) + end + end + + context 'with a parameter containing a reserved word' do + before(:each) do + write_checks(tmpdir, <<~YAML) + --- + version: '2.0.0' + checks: + test_check: + type: puppet-class-parameter + settings: + parameter: module::class + value: foo + YAML + end + + it 'produces a reserved word error' do + expect(lint.errors).to include(match(%r{contains reserved word 'class'})) + end + end + + context 'with a valid parameter' do + before(:each) do + write_checks(tmpdir, <<~YAML) + --- + version: '2.0.0' + checks: + test_check: + type: puppet-class-parameter + settings: + parameter: my_module::my_param + value: foo + YAML + end + + it 'produces no parameter errors' do + expect(lint.errors).not_to include(match(%r{invalid parameter})) + expect(lint.errors).not_to include(match(%r{reserved word})) + end + end + end +end