From c0b2e90be1089ad2cd5deb5b80ac6368bd49a061 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Mon, 11 Dec 2023 10:51:32 -1000 Subject: [PATCH 1/4] Fix running the test suite The rspec-collection_matchers documentation advise to require rspec-collection_matchers form `spec_helper.rb`. This fix: ``` Failure/Error: expect(problems).to have(1).problems NoMethodError: undefined method `have' for # ``` --- spec/spec_helper.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 4aa9e69..7060224 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,3 +1,4 @@ require 'puppet-lint' +require 'rspec/collection_matchers' PuppetLint::Plugins.load_spec_helper From c3d5b6fe53497dd159968ba20438663b635582cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Mon, 11 Dec 2023 14:35:44 -1000 Subject: [PATCH 2/4] Fix checking of warning messages We produce 2 errors in this example. We don't want to check that the first one is present twice: we want to check that each warning is present once. --- spec/puppet-lint/plugins/check_unsafe_interpolations_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/puppet-lint/plugins/check_unsafe_interpolations_spec.rb b/spec/puppet-lint/plugins/check_unsafe_interpolations_spec.rb index b19dee9..22b99f4 100644 --- a/spec/puppet-lint/plugins/check_unsafe_interpolations_spec.rb +++ b/spec/puppet-lint/plugins/check_unsafe_interpolations_spec.rb @@ -45,7 +45,7 @@ class foo { it 'creates two warnings' do expect(problems).to contain_warning(msg) - expect(problems).to contain_warning(msg) + expect(problems).to contain_warning("unsafe interpolation of variable 'bar' in exec command") end end From b35bc5f1b83434fadcbb06f491327725a1062c17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Mon, 11 Dec 2023 14:37:23 -1000 Subject: [PATCH 3/4] Add more checks for onlyif / unless These commands are supposed to be supported, but they are not tested, so add tests to demonstrate that they work as intended. --- .../puppet-lint/plugins/check_unsafe_interpolations_spec.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/spec/puppet-lint/plugins/check_unsafe_interpolations_spec.rb b/spec/puppet-lint/plugins/check_unsafe_interpolations_spec.rb index 22b99f4..0cd088a 100644 --- a/spec/puppet-lint/plugins/check_unsafe_interpolations_spec.rb +++ b/spec/puppet-lint/plugins/check_unsafe_interpolations_spec.rb @@ -33,6 +33,8 @@ class foo { exec { 'bar': command => "echo ${foo} ${bar}", + onlyif => "${baz}", + unless => "${bazz}", } } @@ -40,12 +42,14 @@ class foo { end it 'detects multiple unsafe exec command arguments' do - expect(problems).to have(2).problems + expect(problems).to have(4).problems end it 'creates two warnings' do expect(problems).to contain_warning(msg) expect(problems).to contain_warning("unsafe interpolation of variable 'bar' in exec command") + expect(problems).to contain_warning("unsafe interpolation of variable 'baz' in exec command") + expect(problems).to contain_warning("unsafe interpolation of variable 'bazz' in exec command") end end From 7b7b42d796793f4b8d2864b0f806b58d79527e79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Mon, 11 Dec 2023 14:38:25 -1000 Subject: [PATCH 4/4] Fix detection of insecure interpolations in unless parameter When using the `unless` parameter of an `exec` resource with unsafe string interpolation, the linter should warn about the issue. It happen that it currently doesn't because unless is also a keyword. Adjust the linter to cope with this. --- lib/puppet-lint/plugins/check_unsafe_interpolations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/puppet-lint/plugins/check_unsafe_interpolations.rb b/lib/puppet-lint/plugins/check_unsafe_interpolations.rb index 33838f1..480493c 100644 --- a/lib/puppet-lint/plugins/check_unsafe_interpolations.rb +++ b/lib/puppet-lint/plugins/check_unsafe_interpolations.rb @@ -33,7 +33,7 @@ def check_unsafe_title(title) def check_unsafe_interpolations(command_resources) command_resources[:tokens].each do |token| # Skip iteration if token isn't a command of type :NAME - next unless COMMANDS.include?(token.value) && token.type == :NAME + next unless COMMANDS.include?(token.value) && (token.type == :NAME || token.type == :UNLESS) # Don't check the command if it is parameterised next if parameterised?(token)