feat(modernize): Add PreferRemoteFile, PreferArchiveFile cops + CI fixes by SuhasKumar01 · Pull Request #1051 · chef/cookstyle

SuhasKumar01 · 2026-02-02T06:07:02Z

Summary

Refactored based on maintainer feedback to replace the initial Security Cop with a comprehensive Modernization Suite, plus fixes CI/CD validation failures.

1. Chef/Modernize/PreferRemoteFile


Goal	Discourage shell-based downloads (`curl`/`wget`)
Fix	Suggests native `remote_file` resource
Scope	24 Tests

# ❌ Bad
execute 'curl https://example.com/file.tar.gz -o /tmp/file.tar.gz'

# ✅ Good
remote_file '/tmp/file.tar.gz' do
  source 'https://example.com/file.tar.gz'
  checksum 'sha256checksum'
end

2. Chef/Modernize/PreferArchiveFile


Goal	Discourage shell-based extraction (`tar`, `unzip`, `gunzip`, `gzip`, `7z`)
Fix	Suggests native `archive_file` resource (Chef Infra Client 15.0+)
Scope	30 Tests

# ❌ Bad
execute 'tar -xzf /tmp/app.tar.gz -C /opt'

# ✅ Good
archive_file '/tmp/app.tar.gz' do
  destination '/opt'
end

3. Chef/Correctness/FileUtilsRMRF (Fix)

Fixed offense highlighting to target only FileUtils.rm_rf (not entire expression)
Fixed spec for RuboCop 1.84.2 compatibility (:config metadata)
Added missing doc YAML file

4. CI/CD Fixes

Missing Cop Registrations

Registered 6 cops in config/cookstyle.yml that caused rake validate_config to fail:

Chef/Correctness/FileUtilsRMRF
Chef/Ruby/GemspecRequireRubygems
Chef/Ruby/LegacyPowershellOutMethods
Chef/Ruby/GemspecLicense
Chef/Ruby/RequireNetHttps
Chef/Ruby/UnlessDefinedRequire

Lint Fixes

Removed extra blank line in fileutils_rm_rf_spec.rb (Layout/EmptyLinesAroundBlockBody)
Added .rubocop.yml exclusions for Chef/Ruby/UnlessDefinedRequire on infrastructure files (Rakefile, tasks/**/*)

Deep Dive Cleanup (Standardization)

Changed RSpec.describe → describe in fileutils_rm_rf_spec.rb (match all cop specs)
Standardized @example comments to use # bad / # good across all new cops
Added missing StyleGuide key to FileUtilsRMRF config entry
Created missing doc YAML for Chef/Correctness/FileUtilsRMRF
Aligned VersionAdded to 8.6.5 for all new cops (matching current release)

Design Decisions

No Autocorrect

Both cops intentionally disable autocorrect to prevent breaking:

Piped commands (e.g., curl | bash)
Complex flags (e.g., tar -xzf vs tar -cvf)

Performance

Both use RESTRICT_ON_SEND to limit scanning to shell resources only:

execute, bash, powershell_script, sh, csh, perl, python, ruby, zsh

False Positive Prevention

PreferRemoteFile: Avoids flagging libcurl, curlcache, wgetrc
PreferArchiveFile: Avoids flagging .tar.gz, tarball, stargazer, guitar

Test Results

Component	Count	Status
PreferRemoteFile	24	✅ Passing
PreferArchiveFile	30	✅ Passing
FileUtilsRMRF	3	✅ Passing
`rake validate_config`	—	✅ All Cops found
Full Suite	1022	✅ 0 failures
Cookstyle Lint	548 files	✅ 0 offenses
SonarQube	—	✅ Quality Gate Passed

Files Changed

File	Status
`lib/rubocop/cop/chef/modernize/prefer_remote_file.rb`	➕ Added
`spec/rubocop/cop/chef/modernize/prefer_remote_file_spec.rb`	➕ Added
`lib/rubocop/cop/chef/modernize/prefer_archive_file.rb`	➕ Added
`spec/rubocop/cop/chef/modernize/prefer_archive_file_spec.rb`	➕ Added
`lib/rubocop/cop/chef/correctness/fileutils_rm_rf.rb`	🔧 Fixed
`spec/rubocop/cop/chef/correctness/fileutils_rm_rf_spec.rb`	🔧 Fixed
`config/cookstyle.yml`	📝 Modified (new cops + 6 missing registrations)
`.rubocop.yml`	📝 Modified
`docs-chef-io/assets/cookstyle/cops_chef_modernize_preferremotefile.yml`	➕ Added
`docs-chef-io/assets/cookstyle/cops_chef_modernize_preferarchivefile.yml`	➕ Added
`docs-chef-io/assets/cookstyle/cops_chef_correctness_fileutilsrmrf.yml`	➕ Added

Checklist

dafyddcrosby · 2026-02-03T21:56:37Z

Hmm, this actually seems like it should be two different cops that this PR kind of wants as an end goal.

1st cop - if you see wget/curl/etc in an execute block, you should be suggesting the remote_file resource: https://docs.chef.io/resources/remote_file/
2nd cop - if you're trying to disable TLS in remote_file resources, flag for that.

Trying to track and parse the flags for every single curl/wget tool is a losing game in the long run. But the two cops I suggest are much easier to implement and maintain :-)

SuhasKumar01 · 2026-02-04T07:02:40Z

@dafyddcrosby,
Thanks for the review! That makes perfect sense. Trying to maintain regex for every possible CLI flag is definitely a losing battle long-term.

I agree that Cop #1 (Suggesting remote_file over curl/wget) is the cleaner, more "Chef-idiomatic" approach.

I will refactor this PR right now to implement that specific check—flagging any use of curl/wget in execute blocks and recommending remote_file instead. Updating shortly!

SuhasKumar01 · 2026-02-10T05:50:23Z

@jaymzh
Ready for review!

Based on the feedback that parsing CLI flags is a "losing battle," I have refactored this PR into a complete Modernization Suite that encourages native Chef resources over shell commands.

Refactored: Chef/Modernize/PreferRemoteFile

Goal: Flag curl/wget usage in shell blocks.

Fix: Suggests remote_file resource.

Safety: Autocorrect disabled to preventing breaking piped commands (e.g., curl | bash).

Added: Chef/Modernize/PreferArchiveFile

Goal: Flag tar, unzip, 7z usage.

Fix: Suggests archive_file resource (Chef 15+).

Context: Since curl is usually followed by extraction, this completes the artifact handling pipeline.

Summary:

Performance: Both cops use RESTRICT_ON_SEND to limit scanning to shell resources.

Verification: Added 57 tests covering download patterns, archive patterns, and false positive prevention.

Looking forward to your thoughts on this architectural pivot!

jaymzh · 2026-02-10T08:22:08Z

I haven't reviewed the code yet, I will, but just wanted to clarify here:

Safety: Autocorrect disabled to preventing breaking piped commands (e.g., curl | bash).

Just to be clear... piping commands like that in an execute block is bad practice. If you want to do that, you use an remote_file, which can notify an execute resource (or an archive_file resource). We shouldn't be making rules to make bad code slightly less bad, we should be making rules to make bad code good.

All of these things can and should be done with native resources, not with shell blocks.

But yes, this is a hard one to autocorrect.

jaymzh · 2026-02-10T19:21:28Z

+            return unless curl_or_wget_command?(first_arg.value)
+
+            add_offense(first_arg)
+          end


again, these two are the same.

SuhasKumar01 · 2026-02-11T07:48:39Z

@jaymzh Thanks for the review!

Garbage Files: My apologies, I've removed the extra markdown files and .gitignore.

Refactoring: Good catch on the duplicate methods. I have collapsed check_resource_name and check_command_property into a single helper method to keep it DRY.

Safety: And yes, agreed on the philosophy regarding "bad code." I've left the Autocorrect disabled as discussed.

Pushed the fixes! Ready for another look.

SuhasKumar01 · 2026-02-11T09:10:50Z

@jaymzh

Thanks for the coaching. You were right—having duplicate check methods was sloppy, and "making bad code slightly less bad" is the wrong goal.

Updates:

Refactored (DRY): Collapsed the logic into a single check_offense helper in both cops.
Git Hygiene: I realized I had committed some local config files by mistake. I performed a squash and force-push to clean up the commit history. The PR diff is now clean.
Philosophy: I reviewed the MSG (error messages) to ensure they explain why native resources are better (idempotency/retries), reinforcing the "make code good" mindset.

Ready for re-review!

jaymzh · 2026-02-11T16:52:08Z

some of your tests are failing:

https://github.com/chef/cookstyle/actions/runs/21899029929/job/63260614844?pr=1051

SuhasKumar01 · 2026-02-12T11:33:00Z

@jaymzh

check again now i think everything has been resolved

jaymzh · 2026-02-12T19:07:16Z

you still have many failures.

Chef/Modernize/PreferRemoteFile is being triggered in a Rakefile, which it shouldn't. which is what was failing before.

I see no delta between your last push and this one.

There's line ending failures.

SuhasKumar01 · 2026-02-16T11:30:33Z

@jaymzh

check again now i think everything has been resolved.

…chiveFile) Adds two new cops to discourage shell-based artifact handling in favor of native resources. Signed-off-by: Suhas Kumar <theshahmedia1@gmail.com>

Signed-off-by: Suhas Kumar <theshahmedia1@gmail.com>

… spec - Add 6 missing cop entries to config/cookstyle.yml: Chef/Correctness/FileUtilsRMRF, Chef/Ruby/GemspecRequireRubygems, Chef/Ruby/LegacyPowershellOutMethods, Chef/Ruby/GemspecLicense, Chef/Ruby/RequireNetHttps, Chef/Ruby/UnlessDefinedRequire - Fix FileUtilsRMRF cop to highlight only the method call, not args - Fix FileUtilsRMRF spec to use :config metadata for RuboCop 1.84.2 - validate_config now passes; full suite: 1022 examples, 0 failures Signed-off-by: Suhas Kumar <theshahmedia1@gmail.com>

SuhasKumar01 · 2026-02-18T06:04:20Z

@jaymzh

Hi, I updated my PR several times figuring out every possible error and solved it so please guide further.

- Remove extra blank line in fileutils_rm_rf_spec.rb (Layout/EmptyLinesAroundBlockBody) - Exclude project infrastructure files from Chef/Ruby/UnlessDefinedRequire cop (Rakefile, cleanup_lint_roller.rb, tasks/) in .rubocop.yml Signed-off-by: Suhas Kumar <theshahmedia1@gmail.com>

@example

- Change RSpec.describe -> describe in fileutils_rm_rf_spec.rb (match all cop specs) - Standardize @example comments to use # bad / # good across all new cops - Add missing StyleGuide key to FileUtilsRMRF config entry - Create missing doc YAML for Chef/Correctness/FileUtilsRMRF - Align VersionAdded to 8.6.5 for all new cops (matching current release) Signed-off-by: Suhas Kumar <theshahmedia1@gmail.com>

sonarqubecloud · 2026-02-18T06:40:50Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
23.4% Duplication on New Code

See analysis details on SonarQube Cloud

jaymzh · 2026-02-18T20:40:36Z

Thanks, looking much better. We'll give this a (hopefully) final review at next week's PR review meeting.

SuhasKumar01 · 2026-02-20T06:06:08Z

Thank you very much @jaymzh

dafyddcrosby

Getting closer!

dafyddcrosby · 2026-02-24T22:03:06Z

+                'Native resources handle idempotence and multiple formats natively.'
+
+          # Performance: only trigger on_send for shell resources and command/code property setters
+          RESTRICT_ON_SEND = %i(execute bash sh csh perl python ruby zsh powershell_script command code).freeze


Suggested change

RESTRICT_ON_SEND = %i(execute bash sh csh perl python ruby zsh powershell_script command code).freeze

RESTRICT_ON_SEND = %i(execute bash csh perl python ruby powershell_script command code).freeze

I don't see sh or zsh as documented resources in docs.chef.io, am I missing something here?
This appears to be missing the ksh resource. There's no code resource, so this could end up in a false positive where code isn't a resource property.

dafyddcrosby · 2026-02-24T22:08:53Z

+
+          # Matches extraction tools as whole words.
+          # Excludes filenames like 'file.tar.gz' by ensuring a leading space/start and trailing space/end.
+          ARCHIVE_COMMAND_REGEX = /(?:^|\s)(?:tar|unzip|gunzip|gzip|7z)(?:\s|$)/.freeze


The benefit of having this as a constant is that you can test the strings directly against it

dafyddcrosby · 2026-02-24T22:09:06Z

+
+          # Matches 'curl' or 'wget' as whole words to avoid false positives (e.g., 'libcurl')
+          # Regex: start-of-string OR whitespace, then curl/wget, then whitespace OR end-of-string
+          DOWNLOAD_COMMAND_REGEX = /(?:^|\s)(?:curl|wget)(?:\s|$)/.freeze


Same as above

dafyddcrosby · 2026-02-24T22:09:16Z

+                'Native resources ensure idempotence, support retries, and handle permissions correctly.'
+
+          # Performance: only trigger on_send for shell resources and command/code property setters
+          RESTRICT_ON_SEND = %i(execute bash sh csh perl python ruby zsh powershell_script command code).freeze


Same as above

dafyddcrosby · 2026-02-24T22:12:18Z

+describe RuboCop::Cop::Chef::Modernize::PreferArchiveFile, :config do
+  let(:msg) { 'Use the `archive_file` resource instead of shell-based extraction (`tar`, `unzip`, etc.). Native resources handle idempotence and multiple formats natively.' }
+
+  # ==========================================================================


There's several permutations that are missing. If you loop this through the expected resources+shellout stings instead you can be exhaustive in your tests. See https://www.rubydoc.info/gems/rubocop/RuboCop/RSpec/ExpectOffense for how to do dynamic offense lengths

dafyddcrosby · 2026-02-24T22:12:33Z

+  context 'when using execute with curl' do
+    it 'registers an offense for execute resource with curl command' do
+      expect_offense(<<~RUBY)
+        execute 'curl https://example.com/file.tar.gz -o /tmp/file.tar.gz'


Same as above

jaymzh · 2026-04-21T20:48:22Z

@SuhasKumar01 - following up - do you still want to work on this?

SuhasKumar01 · 2026-04-22T11:24:19Z

i would love too

…

On Wed, Apr 22, 2026 at 2:18 AM Phil Dibowitz ***@***.***> wrote: *jaymzh* left a comment (chef/cookstyle#1051) <#1051?email_source=notifications&email_token=BTF2ZRY3BAKLSS2MBPKM56D4W7NC5A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMRZGE3DOMZYHE4KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJNLQOJPWG33NNVSW45C7N5YGK3S7MNWGSY3L#issuecomment-4291673898> @SuhasKumar01 <https://github.com/SuhasKumar01> - following up - do you still want to work on this? — Reply to this email directly, view it on GitHub <#1051?email_source=notifications&email_token=BTF2ZRY3BAKLSS2MBPKM56D4W7NC5A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMRZGE3DOMZYHE4KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJNLQOJPWG33NNVSW45C7N5YGK3S7MNWGSY3L#issuecomment-4291673898>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BTF2ZR6E33AFYNSBOSHEDHT4W7NC5AVCNFSM6AAAAACTUPLBRWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DEOJRGY3TGOBZHA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

jaymzh · 2026-04-22T17:04:54Z

@SuhasKumar01 - OK, then please address David's comments. I'll keep this open for a bit longer.

SuhasKumar01 requested review from a team and jaymzh as code owners February 2, 2026 06:07

dafyddcrosby added the Status: Waiting on Contributor A pull request that has unresolved requested actions from the author. label Feb 3, 2026

SuhasKumar01 changed the title ~~Add Chef/Security/InsecureCurlWgetDownload cop~~ Add Chef/Modernize/PreferRemoteFile cop Feb 4, 2026

SuhasKumar01 changed the title ~~Add Chef/Modernize/PreferRemoteFile cop~~ Add Chef/Modernize Cops: PreferRemoteFile & PreferArchiveFile (Modernization Suite) Feb 5, 2026

SuhasKumar01 changed the title ~~Add Chef/Modernize Cops: PreferRemoteFile & PreferArchiveFile (Modernization Suite)~~ feat(modernize): Add Modernization Suite (PreferRemoteFile + PreferArchiveFile) Feb 5, 2026

SuhasKumar01 force-pushed the feature/insecure-curl-wget-download branch from d707da9 to e289a9b Compare February 5, 2026 11:00

jaymzh requested changes Feb 10, 2026

View reviewed changes

SuhasKumar01 force-pushed the feature/insecure-curl-wget-download branch from 434b0e5 to 1e6a3ca Compare February 11, 2026 09:10

SuhasKumar01 force-pushed the feature/insecure-curl-wget-download branch from 1e6a3ca to 915bba5 Compare February 12, 2026 11:30

THE-SHAH-MEDIA added 2 commits February 16, 2026 17:01

feat(modernize): Add Modernization Suite (PreferRemoteFile + PreferAr…

b7c4305

…chiveFile) Adds two new cops to discourage shell-based artifact handling in favor of native resources. Signed-off-by: Suhas Kumar <theshahmedia1@gmail.com>

fix: add rubocop cookstyle require, YAML cop docs, and LF line endings

baebdaa

Signed-off-by: Suhas Kumar <theshahmedia1@gmail.com>

SuhasKumar01 force-pushed the feature/insecure-curl-wget-download branch from 481132c to baebdaa Compare February 16, 2026 11:33

SuhasKumar01 changed the title ~~feat(modernize): Add Modernization Suite (PreferRemoteFile + PreferArchiveFile)~~ feat(modernize): Add PreferRemoteFile, PreferArchiveFile cops + CI fixes Feb 18, 2026

THE-SHAH-MEDIA added 2 commits February 18, 2026 11:46

jaymzh self-requested a review February 18, 2026 20:39

jaymzh removed the Status: Waiting on Contributor A pull request that has unresolved requested actions from the author. label Feb 18, 2026

jaymzh requested review from dafyddcrosby and tpowell-progress February 18, 2026 20:39

dafyddcrosby suggested changes Feb 24, 2026

View reviewed changes

jaymzh added the Status: Waiting on Contributor A pull request that has unresolved requested actions from the author. label Apr 21, 2026

	RESTRICT_ON_SEND = %i(execute bash sh csh perl python ruby zsh powershell_script command code).freeze
	RESTRICT_ON_SEND = %i(execute bash csh perl python ruby powershell_script command code).freeze

Conversation

SuhasKumar01 commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

1. Chef/Modernize/PreferRemoteFile

2. Chef/Modernize/PreferArchiveFile

3. Chef/Correctness/FileUtilsRMRF (Fix)

4. CI/CD Fixes

Missing Cop Registrations

Lint Fixes

Deep Dive Cleanup (Standardization)

Design Decisions

No Autocorrect

Performance

False Positive Prevention

Test Results

Files Changed

Checklist

Uh oh!

dafyddcrosby commented Feb 3, 2026

Uh oh!

SuhasKumar01 commented Feb 4, 2026

Uh oh!

SuhasKumar01 commented Feb 10, 2026

Uh oh!

jaymzh commented Feb 10, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SuhasKumar01 commented Feb 11, 2026

Uh oh!

SuhasKumar01 commented Feb 11, 2026

Uh oh!

jaymzh commented Feb 11, 2026

Uh oh!

SuhasKumar01 commented Feb 12, 2026

Uh oh!

jaymzh commented Feb 12, 2026

Uh oh!

SuhasKumar01 commented Feb 16, 2026

Uh oh!

SuhasKumar01 commented Feb 18, 2026

Uh oh!

sonarqubecloud Bot commented Feb 18, 2026

Quality Gate passed

Uh oh!

jaymzh commented Feb 18, 2026

Uh oh!

SuhasKumar01 commented Feb 20, 2026

Uh oh!

dafyddcrosby left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaymzh commented Apr 21, 2026

Uh oh!

SuhasKumar01 commented Apr 22, 2026 via email

Uh oh!

jaymzh commented Apr 22, 2026

Uh oh!

Reviewers

SuhasKumar01 commented Feb 2, 2026 •

edited

Loading