From 851a92edd414a57f0904b8c6febae6e40cf277f4 Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Sat, 26 Sep 2026 00:07:14 -0700 Subject: [PATCH 1/7] Don't depend on OpenStruct in FakeShellOut FakeShellOut, which shell_out returns when running over a train transport connection (chef target mode), built its status with OpenStruct but never required ostruct. It only worked when something else in the process had already loaded it; otherwise it raised NameError. ostruct is also no longer a default gem as of Ruby 3.5, so requiring it would just move the failure to consumers whose Gemfile doesn't list it. Replace it with a small Struct that answers success?. Also teach cspell the words helper.rb already uses, since the spellcheck job only scans files a PR touches. Signed-off-by: Tim Smith --- .expeditor/run_linux_tests.sh | 19 ------------ .expeditor/run_windows_tests.ps1 | 22 -------------- .expeditor/verify.pipeline.yml | 50 -------------------------------- cspell.json | 3 ++ lib/mixlib/shellout/helper.rb | 10 ++++++- 5 files changed, 12 insertions(+), 92 deletions(-) delete mode 100755 .expeditor/run_linux_tests.sh delete mode 100644 .expeditor/run_windows_tests.ps1 delete mode 100644 .expeditor/verify.pipeline.yml diff --git a/.expeditor/run_linux_tests.sh b/.expeditor/run_linux_tests.sh deleted file mode 100755 index 1989688..0000000 --- a/.expeditor/run_linux_tests.sh +++ /dev/null @@ -1,19 +0,0 @@ -#!/bin/bash -# -# This script runs a passed in command, but first setups up the bundler caching on the repo - -set -ue - -export USER="root" -export LANG=C.UTF-8 LANGUAGE=C.UTF-8 - -echo "--- bundle install" -bundle config --local path vendor/bundle -bundle install --jobs=7 --retry=3 - -echo "--- Running Cookstyle" -gem install cookstyle -cookstyle --chefstyle -c .rubocop.yml - -echo "+++ bundle exec task" -bundle exec $@ diff --git a/.expeditor/run_windows_tests.ps1 b/.expeditor/run_windows_tests.ps1 deleted file mode 100644 index d2f4cf4..0000000 --- a/.expeditor/run_windows_tests.ps1 +++ /dev/null @@ -1,22 +0,0 @@ -# Stop script execution when a non-terminating error occurs -$ErrorActionPreference = "Stop" - -# This will run ruby test on windows platform - -Write-Output "--- Bundle install" - -bundle config --local path vendor/bundle -If ($lastexitcode -ne 0) { Exit $lastexitcode } - -bundle install --jobs=7 --retry=3 -If ($lastexitcode -ne 0) { Exit $lastexitcode } - -Write-Output "--- Running Cookstyle" -gem install cookstyle -cookstyle --chefstyle -c .rubocop.yml -If ($lastexitcode -ne 0) { Exit $lastexitcode } - -Write-Output "--- Bundle Execute" - -bundle exec rake -If ($lastexitcode -ne 0) { Exit $lastexitcode } diff --git a/.expeditor/verify.pipeline.yml b/.expeditor/verify.pipeline.yml deleted file mode 100644 index 7edb571..0000000 --- a/.expeditor/verify.pipeline.yml +++ /dev/null @@ -1,50 +0,0 @@ ---- -expeditor: - cached_folders: - - vendor - defaults: - buildkite: - retry: - automatic: - limit: 1 - timeout_in_minutes: 60 - -steps: - -- label: run-lint-and-specs-ruby-3.1 - command: - - .expeditor/run_linux_tests.sh rake - expeditor: - executor: - docker: - image: ruby:3.1 - -- label: run-specs-ruby-3.1-windows - command: - - .expeditor/run_windows_tests.ps1 - expeditor: - executor: - docker: - host_os: windows - shell: ["powershell", "-Command"] - image: rubydistros/windows-2019:3.1 - user: "NT AUTHORITY\\SYSTEM" - -- label: run-lint-and-specs-ruby-3.4 - command: - - .expeditor/run_linux_tests.sh rake - expeditor: - executor: - docker: - image: ruby:3.4 - -- label: run-specs-ruby-3.4-windows - command: - - .expeditor/run_windows_tests.ps1 - expeditor: - executor: - docker: - host_os: windows - shell: ["powershell", "-Command"] - image: rubydistros/windows-2019:3.4 - user: "NT AUTHORITY\\SYSTEM" diff --git a/cspell.json b/cspell.json index e52f6db..8acd354 100644 --- a/cspell.json +++ b/cspell.json @@ -204,6 +204,7 @@ "COLORREF", "COLORSPACE", "COMBOBOX", + "COMMANDINPUT", "commandline", "compat", "COMPOSITECHECK", @@ -682,6 +683,7 @@ "libopenssl", "libruby", "libselinux", + "linelint", "linuxbrew", "linuxmint", "LISTBOX", @@ -1406,6 +1408,7 @@ "subresource", "subresources", "subsession", + "subshell", "SUBSTED", "sunsetting", "SUPPRESSMSGBOXES", diff --git a/lib/mixlib/shellout/helper.rb b/lib/mixlib/shellout/helper.rb index ff45bd5..8a817e2 100644 --- a/lib/mixlib/shellout/helper.rb +++ b/lib/mixlib/shellout/helper.rb @@ -207,6 +207,14 @@ def __env_path_name end class FakeShellOut + # Minimal stand-in for Process::Status. Avoids OpenStruct, which is not + # loaded by default and is no longer a default gem as of Ruby 3.5. + FakeStatus = Struct.new(:success) do + def success? + success + end + end + attr_reader :stdout, :stderr, :exitstatus, :status def initialize(args, options, result) @@ -216,7 +224,7 @@ def initialize(args, options, result) @stderr = result.stderr @exitstatus = result.exit_status @valid_exit_codes = Array(options[:returns] || 0) - @status = OpenStruct.new(success?: (@valid_exit_codes.include? exitstatus)) + @status = FakeStatus.new(@valid_exit_codes.include?(exitstatus)) end def error? From 52e690269fbe187d24ab08d48033237dfd86452a Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Sat, 26 Sep 2026 00:07:14 -0700 Subject: [PATCH 2/7] Modernize the spec suite and cover what could regress Spec infrastructure: - Modern RSpec defaults: RSpec.describe without monkey patching, random ordering, verified partial doubles, --only-failures support. - Fail the run if lib/ emits a Ruby warning. - Drop the DependencyProc ruby-version filter; the gemspec requires 3.1+. - Tag the cgroup specs :requires_root and skip explicitly when cgroup v2 is missing, instead of passing without asserting anything. New coverage: - Helper: the shell_out_compacted argument contract chef's own specs stub against, default locale/PATH injection and overrides, default_env: false, chef provider timeouts, live streaming, and the train transport path (command joining, cwd/input wrapping, FakeShellOut results). - ShellOut: array commands bypass the shell, env reaches the child without leaking into the parent, umask, signal-killed children, sensitive output kept out of exceptions, logger/log_tag/log_level, elevated rejected on unix. - Root-only: uid/gid switching and login simulation, including dropping root's supplementary groups. - Packaging: VERSION matches version.rb, both gemspecs are valid and ship the right files and dependencies, the library loads cleanly with frozen string literals and warnings on, and requiring it does not pull in tmpdir/fileutils (#282). Signed-off-by: Tim Smith --- .gitignore | 1 + .rspec | 4 +- spec/mixlib/shellout/helper_spec.rb | 258 ++++++++++++++++++++++--- spec/mixlib/shellout/packaging_spec.rb | 104 ++++++++++ spec/mixlib/shellout/windows_spec.rb | 2 +- spec/mixlib/shellout_spec.rb | 154 +++++++++++++-- spec/spec_helper.rb | 32 +-- spec/support/dependency_helper.rb | 14 -- spec/support/warnings.rb | 13 ++ 9 files changed, 513 insertions(+), 69 deletions(-) create mode 100644 spec/mixlib/shellout/packaging_spec.rb delete mode 100644 spec/support/dependency_helper.rb create mode 100644 spec/support/warnings.rb diff --git a/.gitignore b/.gitignore index 5064e3c..f6a7f71 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,4 @@ Gemfile.lock */tags *~ vendor/ +spec/examples.txt diff --git a/.rspec b/.rspec index 42b555a..7a2cc1a 100644 --- a/.rspec +++ b/.rspec @@ -1 +1,3 @@ --f documentation --color +--require spec_helper +--format documentation +--color diff --git a/spec/mixlib/shellout/helper_spec.rb b/spec/mixlib/shellout/helper_spec.rb index 8c1136d..85436d5 100644 --- a/spec/mixlib/shellout/helper_spec.rb +++ b/spec/mixlib/shellout/helper_spec.rb @@ -1,42 +1,244 @@ require "spec_helper" require "mixlib/shellout/helper" -require "logger" - -# to use this helper you need to either: -# 1. use mixlib-log which has a trace level -# 2. monkeypatch a trace level into ruby's logger like this -# 3. override the __io_for_live_stream method -# -class Logger - module Severity; TRACE = -1; end - def trace(progname = nil, &block); add(TRACE, nil, progname, &block); end - - def trace?; @level <= TRACE; end -end -describe Mixlib::ShellOut::Helper, ruby: ">= 2.3" do - class TestClass - include Mixlib::ShellOut::Helper +RSpec.describe Mixlib::ShellOut::Helper do + # The helper expects the including class to supply __config, __log and + # __transport_connection. This mirrors how chef and ohai wire it up. + let(:helper_class) do + Class.new do + include Mixlib::ShellOut::Helper + + attr_accessor :__transport_connection, :__log + + def __config + { internal_locale: "C.UTF-8" } + end + end + end + + let(:log) { double("log", trace?: false) } + let(:helper) { helper_class.new.tap { |h| h.__log = log } } + let(:ruby) { RbConfig.ruby } + + describe "#shell_out" do + it "returns a Mixlib::ShellOut that has been run" do + cmd = helper.shell_out(ruby, "-e", "print :hi") + expect(cmd).to be_a(Mixlib::ShellOut) + expect(cmd.stdout).to eq("hi") + expect(cmd.exitstatus).to eq(0) + end + + it "does not raise on a non-zero exit" do + cmd = helper.shell_out(ruby, "-e", "exit 3") + expect(cmd.exitstatus).to eq(3) + expect(cmd.error?).to be(true) + end + + # chef's own specs stub shell_out_compacted, so this calling contract is public in practice + it "flattens, compacts and stringifies arguments before calling shell_out_compacted" do + expect(helper).to receive(:shell_out_compacted).with("foo", "bar", "baz", "1") + helper.shell_out("foo", ["bar", nil, :baz], nil, 1) + end + + it "passes options through to shell_out_compacted" do + expect(helper).to receive(:shell_out_compacted).with("foo", timeout: 5, cwd: "/") + helper.shell_out("foo", timeout: 5, cwd: "/") + end + + it "does not mutate the caller's options hash" do + options = { timeout: 5, default_env: false } + helper.shell_out(ruby, "-e", "exit 0", **options) + expect(options).to eq(timeout: 5, default_env: false) + end + end + + describe "#shell_out!" do + it "flattens, compacts and stringifies arguments before calling shell_out_compacted!" do + expect(helper).to receive(:shell_out_compacted!).with("foo", "bar") + helper.shell_out!(["foo", nil], "bar") + end + + it "returns the command on success" do + expect(helper.shell_out!(ruby, "-e", "print :ok").stdout).to eq("ok") + end + + it "raises ShellCommandFailed on a non-zero exit" do + expect { helper.shell_out!(ruby, "-e", "exit 3") } + .to raise_error(Mixlib::ShellOut::ShellCommandFailed, /received '3'/) + end + + it "honors the returns option" do + expect { helper.shell_out!(ruby, "-e", "exit 3", returns: [0, 3]) }.not_to raise_error + end + end + + describe "default environment" do + def env_passed_to_shellout(**options) + captured = nil + allow(Mixlib::ShellOut).to receive(:new).and_wrap_original do |original, *args, **opts| + captured = opts + original.call(*args, **opts) + end + helper.shell_out(ruby, "-e", "exit 0", **options) + captured + end + + let(:path_key) { windows? ? "Path" : "PATH" } + + it "sets the locale and PATH by default" do + opts = env_passed_to_shellout + expect(opts[:environment]).to include( + "LC_ALL" => "C.UTF-8", + "LANG" => "C.UTF-8", + "LANGUAGE" => "C.UTF-8", + path_key => helper.default_paths + ) + end + + it "lets caller-supplied variables override the defaults" do + opts = env_passed_to_shellout(environment: { "LC_ALL" => "fr_FR.UTF-8", "FOO" => "bar" }) + expect(opts[:environment]).to include("LC_ALL" => "fr_FR.UTF-8", "LANG" => "C.UTF-8", "FOO" => "bar") + end + + it "keeps the :env key when the caller used :env rather than :environment" do + opts = env_passed_to_shellout(env: { "FOO" => "bar" }) + expect(opts).not_to have_key(:environment) + expect(opts[:env]).to include("FOO" => "bar", "LC_ALL" => "C.UTF-8") + end + + it "does not inject anything when default_env is false" do + opts = env_passed_to_shellout(default_env: false) + expect(opts).to eq({}) + end + + it "actually exports the default locale to the child process" do + cmd = helper.shell_out(ruby, "-e", "print ENV['LC_ALL']") + expect(cmd.stdout).to eq("C.UTF-8") + end + end + + describe "timeouts for chef providers" do + # The helper sniffs ancestors by name so it never has to load chef itself. + let(:helper_class) do + provider = Class.new { def self.name = "Chef::Provider" } + Class.new(provider) do + include Mixlib::ShellOut::Helper + + attr_accessor :new_resource, :__log + + def __config = {} + def __transport_connection = nil + end + end + + before { helper.new_resource = resource } - # this is a hash-like object - def __config - {} + context "when the resource has a timeout" do + let(:resource) { double("resource", timeout: 42) } + + it "passes the resource timeout as a float" do + expect(helper).to receive(:shell_out_compacted).with("foo", timeout: 42.0) + helper.shell_out("foo") + end + + it "prefers an explicit timeout option" do + expect(helper).to receive(:shell_out_compacted).with("foo", timeout: 7) + helper.shell_out("foo", timeout: 7) + end end - # this is a train transport connection or nil - def __transport_connection - nil + context "when the resource timeout is nil" do + let(:resource) { double("resource", timeout: nil) } + + it "falls back to 900 seconds" do + expect(helper).to receive(:shell_out_compacted).with("foo", timeout: 900) + helper.shell_out("foo") + end end + end - # this is a logger-like object - def __log - Logger.new(IO::NULL) + it "does not inject a timeout for classes that are not chef providers" do + expect(helper).to receive(:shell_out_compacted).with("foo") + helper.shell_out("foo") + end + + describe "live streaming" do + it "streams to STDOUT when the logger is at trace level" do + allow(log).to receive(:trace?).and_return(true) + expect(helper.send(:__io_for_live_stream)).to be(STDOUT) + end + + it "does not stream otherwise" do + expect(helper.send(:__io_for_live_stream)).to be_nil end end - let(:test_class) { TestClass.new } + # Used by chef target mode, where commands run over a train connection + # instead of locally. + context "with a transport connection" do + let(:result) { Struct.new(:stdout, :stderr, :exit_status).new("out", "err", exit_status) } + let(:exit_status) { 0 } + let(:connection) { double("train connection") } + + before do + helper.__transport_connection = connection + allow(ChefUtils).to receive(:windows?).and_return(false) + # default_paths probes the remote PATH over the connection; keep that out of the way + allow(helper).to receive(:default_paths).and_return("/usr/bin:/bin") + end + + def expect_remote_command(command) + expect(connection).to receive(:run_command).with(command, anything).and_return(result) + end + + it "does not run anything locally" do + allow(connection).to receive(:run_command).and_return(result) + expect(Mixlib::ShellOut).not_to receive(:new) + helper.shell_out("echo", "hi") + end + + it "joins arguments into a single command, quoting any that contain spaces" do + expect_remote_command('echo "hello world"') + helper.shell_out("echo", "hello world") + end + + it "wraps the command in a subshell to honor cwd" do + expect_remote_command("sh -c 'cd /tmp; ls'") + helper.shell_out("ls", cwd: "/tmp") + end - it "works to run a trivial ruby command" do - expect(test_class.shell_out("ruby -e 'exit 0'")).to be_kind_of(Mixlib::ShellOut) + it "feeds input to the command with a heredoc" do + expect_remote_command("cat<<'COMMANDINPUT'\nsome input\nCOMMANDINPUT\n") + helper.shell_out("cat", input: "some input") + end + + it "returns a ShellOut-like result" do + allow(connection).to receive(:run_command).and_return(result) + cmd = helper.shell_out("true") + expect(cmd).to have_attributes(stdout: "out", stderr: "err", exitstatus: 0) + expect(cmd.status.success?).to be(true) + expect(cmd.error?).to be(false) + end + + context "when the remote command fails" do + let(:exit_status) { 2 } + + before { allow(connection).to receive(:run_command).and_return(result) } + + it "reports failure without raising from shell_out" do + cmd = helper.shell_out("false") + expect(cmd.status.success?).to be(false) + expect(cmd.error?).to be(true) + end + + it "raises ShellCommandFailed from shell_out!" do + expect { helper.shell_out!("false") } + .to raise_error(Mixlib::ShellOut::ShellCommandFailed, /Unexpected exit status of 2.*err/) + end + + it "honors the returns option" do + expect { helper.shell_out!("false", returns: [0, 2]) }.not_to raise_error + end + end end end diff --git a/spec/mixlib/shellout/packaging_spec.rb b/spec/mixlib/shellout/packaging_spec.rb new file mode 100644 index 0000000..7aaafba --- /dev/null +++ b/spec/mixlib/shellout/packaging_spec.rb @@ -0,0 +1,104 @@ +require "spec_helper" +require "rbconfig" + +RSpec.describe "mixlib-shellout packaging" do + let(:root) { File.expand_path("../../..", __dir__) } + + def load_gemspec(name) + Dir.chdir(root) { Gem::Specification.load(name) } + end + + describe "version" do + # Expeditor bumps VERSION and then rewrites version.rb with a sed regex + # (.expeditor/update_version.sh). Catch the two drifting apart. + it "matches the VERSION file" do + expect(Mixlib::ShellOut::VERSION).to eq(File.read(File.join(root, "VERSION")).strip) + end + + it "is a valid gem version" do + expect(Gem::Version.correct?(Mixlib::ShellOut::VERSION)).to be(true) + end + end + + describe "mixlib-shellout.gemspec" do + subject(:spec) { load_gemspec("mixlib-shellout.gemspec") } + + it "is valid" do + expect { spec.validate(false) }.not_to raise_error + end + + it "is a pure ruby gem" do + expect(spec.platform).to eq(Gem::Platform::RUBY) + end + + it "ships every library file and the license" do + lib_files = Dir.chdir(root) { Dir.glob("lib/**/*.rb") } + expect(spec.files).to include("LICENSE", *lib_files) + end + + it "does not ship specs or repo tooling" do + expect(spec.files).to all(start_with("lib/").or(eq("LICENSE"))) + end + + it "does not depend on the windows-only gems" do + expect(spec.runtime_dependencies.map(&:name)).to contain_exactly("chef-utils") + end + end + + describe "mixlib-shellout-universal-mingw-ucrt.gemspec" do + subject(:spec) { load_gemspec("mixlib-shellout-universal-mingw-ucrt.gemspec") } + + it "is valid" do + expect { spec.validate(false) }.not_to raise_error + end + + it "targets the ucrt windows platform" do + expect(spec.platform.to_s).to eq("x64-mingw-ucrt") + end + + it "adds the win32 dependencies used by lib/mixlib/shellout/windows.rb" do + expect(spec.runtime_dependencies.map(&:name)) + .to contain_exactly("chef-utils", "win32-process", "wmi-lite", "ffi-win32-extensions") + end + + it "ships the same files as the ruby platform gem" do + expect(spec.files).to match_array(load_gemspec("mixlib-shellout.gemspec").files) + end + end + + # These run in a fresh interpreter: this process has already loaded plenty + # of stdlib, so checking $LOADED_FEATURES here would prove nothing. + describe "requiring the library" do + def ruby_in_clean_process(*flags, code) + cmd = Mixlib::ShellOut.new(RbConfig.ruby, *flags, "-I", File.join(root, "lib"), "-e", code) + cmd.run_command + expect(cmd.stderr).to eq("") + cmd.error! + cmd.stdout + end + + # Regression guard for https://github.com/chef/mixlib-shellout/pull/282: + # every chef/ohai/inspec process pays for whatever this gem loads. + it "does not load tmpdir or fileutils", :unix_only do + # Diff against a baseline: under `bundle exec` bundler has already loaded its own vendored fileutils. + out = ruby_in_clean_process(<<~RUBY) + before = $LOADED_FEATURES.dup + require "mixlib/shellout" + Mixlib::ShellOut.new("true").run_command.error! + print ($LOADED_FEATURES - before).grep(%r{/(tmpdir|fileutils)\\.rb$}).join(",") + RUBY + expect(out).to eq("") + end + + it "loads and runs a command with frozen string literals and warnings enabled" do + out = ruby_in_clean_process("-W:deprecated", "-w", "--enable-frozen-string-literal", <<~RUBY) + require "mixlib/shellout" + require "mixlib/shellout/helper" + cmd = Mixlib::ShellOut.new(#{RbConfig.ruby.dump}, "-e", "print :ok") + cmd.live_stream = String.new + print cmd.run_command.stdout + RUBY + expect(out).to eq("ok") + end + end +end diff --git a/spec/mixlib/shellout/windows_spec.rb b/spec/mixlib/shellout/windows_spec.rb index 2b273a6..5bc52c7 100644 --- a/spec/mixlib/shellout/windows_spec.rb +++ b/spec/mixlib/shellout/windows_spec.rb @@ -2,7 +2,7 @@ # FIXME: these are stubby enough unit tests that they almost run under unix, but the # Mixlib::ShellOut object does not mixin the Windows behaviors when running on unix. -describe "Mixlib::ShellOut::Windows", :windows_only do +RSpec.describe "Mixlib::ShellOut::Windows", :windows_only do describe "Utils" do describe ".should_run_under_cmd?" do diff --git a/spec/mixlib/shellout_spec.rb b/spec/mixlib/shellout_spec.rb index 08a12f1..94d292d 100644 --- a/spec/mixlib/shellout_spec.rb +++ b/spec/mixlib/shellout_spec.rb @@ -3,7 +3,7 @@ require "logger" require "timeout" -describe Mixlib::ShellOut do +RSpec.describe Mixlib::ShellOut do let(:shell_cmd) { options ? shell_cmd_with_options : shell_cmd_without_options } let(:executed_cmd) { shell_cmd.tap(&:run_command) } let(:stdout) { executed_cmd.stdout } @@ -1567,6 +1567,101 @@ def ruby_wo_shell(code) end end end + + context "with an array of command and args", :unix_only do + # Nothing here should be interpreted by a shell: this is what makes the + # array form safe to use with untrusted arguments. + let(:shell_cmd) { Mixlib::ShellOut.new("echo", "$HOME", "|", "cat", ";", "exit 3") } + + it "passes the arguments to the command verbatim without a shell" do + expect(stdout).to eql("$HOME | cat ; exit 3\n") + expect(executed_cmd.exitstatus).to eql(0) + end + end + + context "with environment variables" do + let(:ruby_code) { %q{print ENV["MIXLIB_SHELLOUT_SPEC"]} } + let(:options) { { environment: { "MIXLIB_SHELLOUT_SPEC" => "from-parent" } } } + + it "sets them in the child" do + expect(stdout).to eql("from-parent") + end + + it "does not leak them into the parent's environment" do + executed_cmd + expect(ENV).not_to have_key("MIXLIB_SHELLOUT_SPEC") + end + end + + context "with a umask", :unix_only do + let(:ruby_code) { %q{printf("%04o", File.umask)} } + let(:options) { { umask: "0027" } } + + it "applies it to the child" do + expect(stdout).to eql("0027") + end + end + + context "when the child is killed by a signal", :unix_only do + let(:ruby_code) { "Process.kill(:KILL, Process.pid)" } + + it "has no exit status" do + expect(executed_cmd.exitstatus).to be_nil + expect(executed_cmd.status.termsig).to eql(9) + end + + it "is treated as an error" do + expect(executed_cmd.error?).to be(true) + expect { executed_cmd.error! }.to raise_error(Mixlib::ShellOut::ShellCommandFailed) + end + end + + context "with sensitive output" do + let(:ruby_code) { %q{puts "s3cr3t"; exit 1} } + let(:options) { { sensitive: true } } + + it "still captures stdout" do + expect(stdout).to include("s3cr3t") + end + + it "keeps the output out of the exception message" do + expect { executed_cmd.error! }.to raise_error(Mixlib::ShellOut::ShellCommandFailed) do |e| + expect(e.message).not_to include("s3cr3t") + expect(e.message).to include("STDOUT/STDERR suppressed for sensitive resource") + end + end + end + + context "with a logger" do + let(:log_output) { StringIO.new } + let(:logger) { Logger.new(log_output).tap { |l| l.level = Logger::INFO } } + let(:ruby_code) { "exit 0" } + + context "with a log level and tag" do + let(:options) { { logger:, log_level: :info, log_tag: "my-tag" } } + + it "logs the command before running it" do + executed_cmd + expect(log_output.string).to include("INFO -- : my-tag sh(#{cmd})") + end + end + + context "with the default log level" do + let(:options) { { logger: } } + + it "logs at debug" do + executed_cmd + expect(log_output.string).to be_empty + end + end + end + end + + context "with the elevated option on unix", :unix_only do + it "is rejected" do + expect { Mixlib::ShellOut.new("true", elevated: true) } + .to raise_error(Mixlib::ShellOut::InvalidCommandOption, /elevated/) + end end context "when running under *nix", :requires_root, :unix_only do @@ -1588,31 +1683,64 @@ def ruby_wo_shell(code) expect(running_user).to eql(user.to_s) end end + + # Ordering matters in the child: groups must be dropped while still root, + # before the uid switch. These catch that being reordered. + context "when user and group are given as ids" do + let(:nobody) { Etc.getpwnam("nobody") } + let(:cmd) { "id -u; id -g" } + let(:options) { { user: nobody.uid, group: nobody.gid } } + + it "should drop to that uid and gid" do + expect(shell_cmd.run_command.stdout.split).to eql([nobody.uid.to_s, nobody.gid.to_s]) + end + end + + context "when simulating a login" do + let(:nobody) { Etc.getpwnam("nobody") } + let(:cmd) { %q{echo "$USER $LOGNAME $HOME"; id -G} } + let(:options) { { user: "nobody", login: true } } + let(:output) { shell_cmd.run_command.stdout.lines.map(&:chomp) } + let(:expected_groups) do + secondary = [] + Etc.group { |g| secondary << g.gid if g.mem.include?("nobody") } + [nobody.gid, *secondary].uniq + end + + it "should set the login environment for the user" do + expect(output.first).to eql("nobody nobody #{nobody.dir}") + end + + it "should use only the user's groups, dropping root's" do + expect(output.last.split.map(&:to_i)).to match_array(expected_groups) + end + end end - context "when running on a cgroup", :linux_only do + context "when running on a cgroup", :linux_only, :requires_root do let(:cmd) { "cat /proc/self/cgroup | cut -c 4-" } let(:options) { { cgroup: } } - let(:cgroupv2_supported) { File.read("/proc/mounts").match(%r{^cgroup2 /sys/fs/cgroup}) } + let(:running_cgroup) { shell_cmd.run_command.stdout.chomp } + + before do + skip "cgroup v2 is not mounted at /sys/fs/cgroup" unless File.read("/proc/mounts").match?(%r{^cgroup2 /sys/fs/cgroup}) + end context "when cgroup exists" do let(:cgroup) { "#{File.read("/proc/self/cgroup")[%r{(/.*)$}, 1]}" } - let(:running_cgroup) { shell_cmd.run_command.stdout.chomp } + it "should run the process under that cgroup" do - if cgroupv2_supported - expect(running_cgroup).to eql(cgroup.to_s) - end + expect(running_cgroup).to eql(cgroup.to_s) end end context "when cgroup does not exist" do - let(:cgroup) { "#{File.read("/proc/self/cgroup")[%r{(/.*)/[^/]+$}, 1]}/test" } - let(:running_cgroup) { shell_cmd.run_command.stdout.chomp } + let(:cgroup) { "#{File.read("/proc/self/cgroup")[%r{(/.*)/[^/]+$}, 1]}/mixlib-shellout-test" } + + after { Dir.rmdir("/sys/fs/cgroup/#{cgroup}") if Dir.exist?("/sys/fs/cgroup/#{cgroup}") } + it "should create the cgroup and run the process under it" do - if cgroupv2_supported - expect(running_cgroup).to eql(cgroup.to_s) - Dir.rmdir("/sys/fs/cgroup/#{cgroup}") - end + expect(running_cgroup).to eql(cgroup.to_s) end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 420d9f4..ea5b401 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,3 +1,4 @@ +require_relative "support/warnings" require "mixlib/shellout" require "tmpdir" @@ -5,25 +6,32 @@ require "timeout" # Load everything from spec/support -Dir["spec/support/**/*.rb"].each { |f| require File.expand_path(f) } +Dir[File.join(__dir__, "support", "**", "*.rb")].sort.each { |f| require f } RSpec.configure do |config| - config.mock_with :rspec - config.filter_run focus: true - config.filter_run_excluding external: true + config.expect_with :rspec do |c| + c.syntax = :expect + c.include_chain_clauses_in_custom_matcher_descriptions = true + end + + config.mock_with :rspec do |mocks| + mocks.verify_partial_doubles = true + end + + config.shared_context_metadata_behavior = :apply_to_host_groups + config.disable_monkey_patching! + config.warnings = true - # Add jruby filters here + config.filter_run_when_matching :focus + config.filter_run_excluding external: true config.filter_run_excluding windows_only: true unless windows? config.filter_run_excluding unix_only: true unless unix? config.filter_run_excluding linux_only: true unless linux? config.filter_run_excluding requires_root: true unless root? - config.filter_run_excluding ruby: DependencyProc.with(RUBY_VERSION) - config.run_all_when_everything_filtered = true - - config.warnings = true + # Enables `rspec --only-failures` and `rspec --next-failure` + config.example_status_persistence_file_path = "spec/examples.txt" - config.expect_with :rspec do |c| - c.syntax = :expect - end + config.order = :random + Kernel.srand config.seed end diff --git a/spec/support/dependency_helper.rb b/spec/support/dependency_helper.rb deleted file mode 100644 index f4f1af8..0000000 --- a/spec/support/dependency_helper.rb +++ /dev/null @@ -1,14 +0,0 @@ -class DependencyProc < Proc - attr_accessor :present - - def self.with(present) - provided = Gem::Version.new(present.dup) - new do |required| - !Gem::Requirement.new(required).satisfied_by?(provided) - end.tap { |l| l.present = present } - end - - def inspect - "\"#{present}\"" - end -end diff --git a/spec/support/warnings.rb b/spec/support/warnings.rb new file mode 100644 index 0000000..502cca1 --- /dev/null +++ b/spec/support/warnings.rb @@ -0,0 +1,13 @@ +# Fail the run if the library itself emits a Ruby warning (method redefinition, +# unused variables, deprecated APIs, etc). Warnings from other gems are left alone. +module LibWarningsAreErrors + LIB_DIR = File.expand_path("../../lib", __dir__) + + def warn(message, *args, **kwargs) + raise "Ruby warning emitted from lib/: #{message}" if message.include?(LIB_DIR) + + super + end +end + +Warning.singleton_class.prepend(LibWarningsAreErrors) From c86f4c5d476120a4ef1e5731df7b40db6e4ee98d Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Sat, 26 Sep 2026 00:07:14 -0700 Subject: [PATCH 3/7] Run tests on GitHub Actions and keep actions up to date Replace the Buildkite verify pipeline with a GitHub Actions workflow: - Linux, macOS and Windows on every supported Ruby (3.1 through 4.0), plus ruby-head as an allowed failure. - The full suite as root on Linux, so user/group switching, login simulation and cgroup specs actually run. - A run with --enable-frozen-string-literal forced on for everything. - Build both gems with --strict, install the result in an isolated GEM_HOME and smoke test it outside the source tree. Pin every action to a commit SHA with a version comment and add dependabot (with a cooldown) for actions and bundler. Update actions that had fallen behind or tracked a branch. Skip the DCO check on dependabot PRs, since dependabot cannot sign off. Give the gemspec a real description so gem build --strict passes, and drop Gemfile branches for Ruby 3.0, which is no longer supported. Signed-off-by: Tim Smith --- .expeditor/config.yml | 5 -- .github/dependabot.yml | 25 +++++++ .github/workflows/allchecks.yml | 2 +- .github/workflows/ci.yml | 116 ++++++++++++++++++++++++++++++++ .github/workflows/dco.yml | 6 +- .github/workflows/lint.yml | 40 ++++++----- Gemfile | 11 --- mixlib-shellout.gemspec | 2 +- 8 files changed, 169 insertions(+), 38 deletions(-) create mode 100644 .github/dependabot.yml create mode 100644 .github/workflows/ci.yml diff --git a/.expeditor/config.yml b/.expeditor/config.yml index 5a4cd54..da8335b 100644 --- a/.expeditor/config.yml +++ b/.expeditor/config.yml @@ -46,8 +46,3 @@ subscriptions: actions: - built_in:rollover_changelog - built_in:publish_rubygems - -pipelines: - - verify: - description: Pull Request validation tests - public: true diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..ef8b519 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,25 @@ +--- +version: 2 +updates: + # Actions are pinned to commit SHAs with a version comment; dependabot + # updates both together. + - package-ecosystem: github-actions + directory: / + schedule: + interval: weekly + cooldown: + default-days: 7 + groups: + github-actions: + patterns: + - "*" + + - package-ecosystem: bundler + directory: / + schedule: + interval: weekly + cooldown: + default-days: 7 + groups: + development-dependencies: + dependency-type: development diff --git a/.github/workflows/allchecks.yml b/.github/workflows/allchecks.yml index c51c3d6..73f7814 100644 --- a/.github/workflows/allchecks.yml +++ b/.github/workflows/allchecks.yml @@ -10,7 +10,7 @@ jobs: checks: read contents: read steps: - - uses: wechuli/allcheckspassed@v1 + - uses: wechuli/allcheckspassed@e4240aa9cc76fd6828ce27a71ad16c406c25adb3 # v2.5.0 with: # This seems to be working lately even for external # contributors, so maybe we don't need to exclude it? diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..8495f2f --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,116 @@ +--- +name: CI + +on: + pull_request: + push: + branches: + - main + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +env: + BUNDLE_WITHOUT: debug + +jobs: + test: + name: Ruby ${{ matrix.ruby }} on ${{ matrix.os }} + runs-on: ${{ matrix.os }} + timeout-minutes: 20 + continue-on-error: ${{ matrix.experimental || false }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest, windows-latest] + # keep in sync with required_ruby_version in the gemspec + ruby: ["3.1", "3.2", "3.3", "3.4", "4.0"] + include: + # early warning for the next ruby release; allowed to fail + - os: ubuntu-latest + ruby: head + experimental: true + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: ruby/setup-ruby@14594264cd68ce8a2345dd349bc3d138a4ef85c8 # v1.327.0 + with: + ruby-version: ${{ matrix.ruby }} + bundler-cache: true + - run: bundle exec rspec + + # User/group switching, login simulation and cgroups only run as root. + test-root: + name: Ruby ${{ matrix.ruby }} on ubuntu-latest as root + runs-on: ubuntu-latest + timeout-minutes: 20 + strategy: + fail-fast: false + matrix: + ruby: ["3.1", "4.0"] + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: ruby/setup-ruby@14594264cd68ce8a2345dd349bc3d138a4ef85c8 # v1.327.0 + with: + ruby-version: ${{ matrix.ruby }} + bundler-cache: true + - run: sudo --preserve-env env "PATH=$PATH" bundle exec rspec + + # Ruby is moving towards frozen string literals by default. Run everything, + # dependencies included, with them forced on so we find out early. + frozen-string-literals: + name: Frozen string literals + runs-on: ubuntu-latest + timeout-minutes: 20 + env: + RUBYOPT: --enable-frozen-string-literal + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: ruby/setup-ruby@14594264cd68ce8a2345dd349bc3d138a4ef85c8 # v1.327.0 + with: + ruby-version: "4.0" + bundler-cache: true + - run: bundle exec rspec + + # Build the gems exactly as a release would, install the result somewhere + # isolated and make sure it works without the source tree. + package: + name: Build and install gem + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: ruby/setup-ruby@14594264cd68ce8a2345dd349bc3d138a4ef85c8 # v1.327.0 + with: + ruby-version: "4.0" + - name: Build gems + run: | + gem build mixlib-shellout.gemspec --strict + gem build mixlib-shellout-universal-mingw-ucrt.gemspec --strict + - name: Install and smoke test outside the source tree + run: | + export GEM_HOME="$RUNNER_TEMP/gems" GEM_PATH="$RUNNER_TEMP/gems" + gem install --no-document "./mixlib-shellout-$(cat VERSION).gem" + cd "$RUNNER_TEMP" + ruby - <<'RUBY' + require "mixlib/shellout" + require "mixlib/shellout/helper" + require "mixlib/shellout/version" + abort "not loaded from the installed gem" unless Gem.loaded_specs.key?("mixlib-shellout") + cmd = Mixlib::ShellOut.new("echo", "hello").run_command + cmd.error! + abort "unexpected output: #{cmd.stdout.inspect}" unless cmd.stdout == "hello\n" + puts "mixlib-shellout #{Mixlib::ShellOut::VERSION} OK" + RUBY diff --git a/.github/workflows/dco.yml b/.github/workflows/dco.yml index 0ae9f32..0d8fd4c 100644 --- a/.github/workflows/dco.yml +++ b/.github/workflows/dco.yml @@ -10,14 +10,16 @@ jobs: pull-requests: read runs-on: ubuntu-latest name: DCO Check + # dependabot cannot sign off its commits + if: github.event.pull_request.user.login != 'dependabot[bot]' steps: - name: Get PR Commits - uses: actionshub/get-pr-commits@main + uses: actionshub/get-pr-commits@0f1d778e95718cdf9a80f57d36c0a8754e874fa1 # v2.0.0 id: 'get-pr-commits' with: token: ${{ secrets.GITHUB_TOKEN }} - name: DCO Check - uses: actionshub/dco@main + uses: actionshub/dco@624651527997baebfe5fd772d216f9c77bfd40f3 # v2.0.0 with: commits: ${{ steps.get-pr-commits.outputs.commits }} allow-obvious-fix-label: "obvious-fix" diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 90ae24c..4ec301e 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -7,40 +7,44 @@ on: branches: - main +permissions: + contents: read + concurrency: - group: lint-${{ github.event.pull_request.number || github.run_id }} - cancel-in-progress: true + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: cookstyle: runs-on: ubuntu-latest env: - BUNDLE_WITHOUT: ruby_shadow:packaging + BUNDLE_WITHOUT: debug steps: - - uses: actions/checkout@v6 - - uses: ruby/setup-ruby@v1 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - ruby-version: 3.4 - bundler-cache: false - - uses: r7kamura/rubocop-problem-matchers-action@v1 # this shows the failures in the PR - - run: | - bundle install - bundle exec cookstyle --chefstyle -c .rubocop.yml + persist-credentials: false + - uses: ruby/setup-ruby@14594264cd68ce8a2345dd349bc3d138a4ef85c8 # v1.327.0 + with: + ruby-version: "4.0" + bundler-cache: true + - uses: r7kamura/rubocop-problem-matchers-action@59f1a0759f50cc2649849fd850b8487594bb5a81 # v1.2.2 + - run: bundle exec cookstyle --chefstyle -c .rubocop.yml spellcheck: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - run: | curl --location 'https://raw.githubusercontent.com/chef/chef_dictionary/main/chef.txt' --output chef_dictionary.txt - - uses: streetsidesoftware/cspell-action@v8.4.0 + - uses: streetsidesoftware/cspell-action@6f3c77c1406bc930f944ba97e9801a22e42caf58 # v9.1.0 linelint: runs-on: ubuntu-latest name: Check if all files end in newline steps: - - name: Checkout - uses: actions/checkout@v6 - - name: Linelint - uses: fernandrone/linelint@master - id: linelint + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: fernandrone/linelint@7907a5dca0c28ea7dd05c6d8d8cacded713aca11 # 0.0.6 diff --git a/Gemfile b/Gemfile index 1d1fbbb..e3260d9 100644 --- a/Gemfile +++ b/Gemfile @@ -3,11 +3,6 @@ source "https://rubygems.org" gemspec name: "mixlib-shellout" gem "win32-process", "~> 0.9" -# for ruby 3.0, install older ffi before -# installing ffi-requiring stuff -if Gem::Version.new(RUBY_VERSION) < Gem::Version.new("3.1") - gem "ffi", "< 1.17.0" -end gem "ffi-win32-extensions", "~> 1.0.4" gem "wmi-lite", "~> 1.0.7" gem "logger" @@ -20,12 +15,6 @@ end group :debug do gem "pry" - # version lock for old ruby 3.0 - once we're off - # of 3.0, remove this line, let pry-byebug pull - # in whatever it wants - if Gem::Version.new(RUBY_VERSION) < Gem::Version.new("3.1") - gem "byebug", "~> 11.1" - end gem "pry-byebug" gem "rb-readline" end diff --git a/mixlib-shellout.gemspec b/mixlib-shellout.gemspec index d554437..4c9275d 100644 --- a/mixlib-shellout.gemspec +++ b/mixlib-shellout.gemspec @@ -10,7 +10,7 @@ Gem::Specification.new do |s| s.version = Mixlib::ShellOut::VERSION s.platform = Gem::Platform::RUBY s.summary = "Run external commands on Unix or Windows" - s.description = s.summary + s.description = "Run external commands on Unix or Windows, capturing stdout, stderr and exit status, with control over environment, working directory, user, group, umask and timeout." s.author = "Chef Software Inc." s.email = "info@chef.io" s.homepage = "https://github.com/chef/mixlib-shellout" From 4585b11894d5c5bd18ff06f51389f975feab07db Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Sat, 26 Sep 2026 00:07:15 -0700 Subject: [PATCH 4/7] Refresh README badges and links Swap the Buildkite badge for GitHub Actions, use shields.io for the gem version and add a license badge. Link docs instead of bare URLs, point the "see also" section at Process.spawn/Open3, use https for the license URL, and document how to run the specs, including the root-only ones. Fix the CONTRIBUTING link that still pointed at master, and update the copilot instructions for the new CI layout. Signed-off-by: Tim Smith --- .github/copilot-instructions.md | 13 +++++++------ CONTRIBUTING.md | 2 +- README.md | 31 ++++++++++++++++++++++++++----- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 9493aee..6ae0192 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -10,15 +10,15 @@ mixlib-shellout/ ├── .expeditor/ # Expeditor CI/CD configuration │ ├── config.yml # Main Expeditor configuration -│ ├── verify.pipeline.yml # Build pipeline definition -│ ├── run_linux_tests.sh # Linux test runner -│ ├── run_windows_tests.ps1 # Windows test runner │ └── update_version.sh # Version update script ├── .github/ │ ├── CODEOWNERS # Code ownership definitions │ ├── ISSUE_TEMPLATE/ # Issue templates +│ ├── dependabot.yml # Keeps actions and gems up to date │ ├── workflows/ # GitHub Actions workflows -│ │ └── ci-main-pull-request-checks.yml +│ │ ├── ci.yml # Specs across OS/Ruby matrix, as root, packaging +│ │ ├── lint.yml # Cookstyle, spellcheck, linelint +│ │ └── ci-main-pull-request-stub-*.yml # Shared chef security/quality checks │ └── copilot-instructions.md # This file ├── lib/mixlib/ │ ├── shellout.rb # Main ShellOut class @@ -36,6 +36,7 @@ mixlib-shellout/ │ │ ├── shellout_spec.rb # Main test file │ │ └── shellout/ │ │ ├── helper_spec.rb # Helper tests +│ │ ├── packaging_spec.rb # Gemspec, version and load-time tests │ │ └── windows_spec.rb # Windows-specific tests │ └── support/ # Test support files ├── vendor/bundle/ # Bundled gems (gitignored in production) @@ -128,12 +129,12 @@ Signed-off-by: Your Name The repository uses **Expeditor** for automated CI/CD: - **Main config**: `.expeditor/config.yml` -- **Build pipeline**: `.expeditor/verify.pipeline.yml` - **Notifications**: Sent to `#chef-found-notify` Slack channel - **Auto-versioning**: Supports major/minor version bumps via labels ### GitHub Actions -- **Workflow**: `.github/workflows/ci-main-pull-request-checks.yml` +- **Tests**: `.github/workflows/ci.yml` runs specs on Linux, macOS and Windows for every supported Ruby, as root on Linux, with frozen string literals, and against the built gem +- **Workflow**: `.github/workflows/ci-main-pull-request-stub-*.yml` - **Triggers**: Pull requests and pushes to `main` and `release/**` branches - **Features**: Complexity checks, TruffleHog scanning, SBOM generation diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 03fdbfa..a0bbb96 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1 +1 @@ -Please refer to https://github.com/chef/chef/blob/master/CONTRIBUTING.md +Please refer to https://github.com/chef/chef/blob/main/CONTRIBUTING.md diff --git a/README.md b/README.md index 4471d9c..3c24448 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,8 @@ # Mixlib::ShellOut -[![Build Status](https://badge.buildkite.com/7051b7b35cc19076c35a6e6a9e996807b0c14475ca3f3acd86.svg?branch=main)](https://buildkite.com/chef-oss/chef-mixlib-shellout-master-verify) [![Gem Version](https://badge.fury.io/rb/mixlib-shellout.svg)](https://badge.fury.io/rb/mixlib-shellout) +[![CI](https://github.com/chef/mixlib-shellout/actions/workflows/ci.yml/badge.svg?branch=main)](https://github.com/chef/mixlib-shellout/actions/workflows/ci.yml) +[![Gem Version](https://img.shields.io/gem/v/mixlib-shellout)](https://rubygems.org/gems/mixlib-shellout) +[![License](https://img.shields.io/github/license/chef/mixlib-shellout)](LICENSE) Provides a simplified interface to shelling out while still collecting both standard out and standard error and providing full control over environment, working directory, uid, gid, etc. @@ -33,7 +35,7 @@ Raise an exception if it didn't exit with 0 ``` ### Advanced Shellout -In addition to the command to run there are other options that can be set to change the shellout behavior. The complete list of options can be found here: https://github.com/chef/mixlib-shellout/blob/main/lib/mixlib/shellout.rb +In addition to the command to run there are other options that can be set to change the shellout behavior. The complete list of options is documented in [`lib/mixlib/shellout.rb`](lib/mixlib/shellout.rb). Run a command as the `www` user with no extra ENV settings from `/tmp` with a 1s timeout @@ -73,8 +75,27 @@ Invoke "whoami.exe" with elevated privileges: Mixlib::ShellOut does a standard fork/exec on Unix, and uses the Win32 API on Windows. There is not currently support for JRuby. ## See Also -- `Process.spawn` in Ruby 1.9+ -- [https://github.com/rtomayko/posix-spawn](https://github.com/rtomayko/posix-spawn) +- Ruby's built-in [`Process.spawn`](https://docs.ruby-lang.org/en/master/Process.html#method-c-spawn) and [`Open3`](https://docs.ruby-lang.org/en/master/Open3.html) + +## Development + +```shell +bundle install +bundle exec rake # cookstyle + specs +bundle exec rspec # specs only +bundle exec rspec --only-failures +``` + +Specs that switch users, groups or cgroups are tagged `:requires_root` and are +skipped unless run as root. On Linux you can run them with: + +```shell +sudo --preserve-env env "PATH=$PATH" bundle exec rspec +``` + +CI runs the suite on Linux, macOS and Windows across every supported Ruby, as +root on Linux, with frozen string literals forced on, and against the built gem. +See [`.github/workflows/ci.yml`](.github/workflows/ci.yml). ## Contributing @@ -88,7 +109,7 @@ Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 + https://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, From f20f8923e97cfca9a20d33c949589f71aa8ea72a Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Sat, 26 Sep 2026 00:13:54 -0700 Subject: [PATCH 5/7] Switch to the user's primary group when simulating a login With login: true and a user but no explicit group, #gid resolves the user's primary group, but set_group only acted when group was set. The child switched uid and cleared supplementary groups but kept the parent's primary group, so a command run as "nobody" from a root process still had gid 0. Check #gid instead. Without login and without a group #gid is still nil, so that case is unchanged. Signed-off-by: Tim Smith --- lib/mixlib/shellout/unix.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/mixlib/shellout/unix.rb b/lib/mixlib/shellout/unix.rb index 3010da2..1e3cd2d 100644 --- a/lib/mixlib/shellout/unix.rb +++ b/lib/mixlib/shellout/unix.rb @@ -149,10 +149,13 @@ def set_user end end + # gid also resolves the user's primary group when simulating a login, so + # check it rather than group or a login would keep the parent's group. def set_group - if group - Process.egid = gid - Process.gid = gid + new_gid = gid + if new_gid + Process.egid = new_gid + Process.gid = new_gid end end From 2ab9e3e98a2fc30c0f6f26b11085ae0690dd65ac Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Sat, 26 Sep 2026 00:13:54 -0700 Subject: [PATCH 6/7] Depend on win32ole for Ruby 4.0 on Windows wmi-lite requires win32ole, which is a bundled rather than default gem as of Ruby 4.0, so it can no longer be required under bundler unless it is declared. mixlib-shellout loads wmi-lite when a command times out, to kill the process tree, so on Ruby 4.0 every timeout raised LoadError instead of CommandTimeout and left the process tree running. Add it to the Windows gem's dependencies and the Gemfile. Signed-off-by: Tim Smith --- Gemfile | 2 ++ mixlib-shellout-universal-mingw-ucrt.gemspec | 2 ++ spec/mixlib/shellout/packaging_spec.rb | 7 +++++-- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index e3260d9..245f538 100644 --- a/Gemfile +++ b/Gemfile @@ -5,6 +5,8 @@ gemspec name: "mixlib-shellout" gem "win32-process", "~> 0.9" gem "ffi-win32-extensions", "~> 1.0.4" gem "wmi-lite", "~> 1.0.7" +# wmi-lite needs win32ole, which stopped being a default gem in Ruby 4.0 +gem "win32ole" if Gem.win_platform? gem "logger" group :test do diff --git a/mixlib-shellout-universal-mingw-ucrt.gemspec b/mixlib-shellout-universal-mingw-ucrt.gemspec index 3972f3a..3aac3a3 100644 --- a/mixlib-shellout-universal-mingw-ucrt.gemspec +++ b/mixlib-shellout-universal-mingw-ucrt.gemspec @@ -3,6 +3,8 @@ gemspec = instance_eval(File.read(File.expand_path("mixlib-shellout.gemspec", __ gemspec.platform = Gem::Platform.new("x64-mingw-ucrt") gemspec.add_dependency "win32-process", "~> 0.9" gemspec.add_dependency "wmi-lite", "~> 1.0" +# wmi-lite needs win32ole, which stopped being a default gem in Ruby 4.0 +gemspec.add_dependency "win32ole" gemspec.add_dependency "ffi-win32-extensions", "~> 1.0.3" gemspec diff --git a/spec/mixlib/shellout/packaging_spec.rb b/spec/mixlib/shellout/packaging_spec.rb index 7aaafba..e5607c2 100644 --- a/spec/mixlib/shellout/packaging_spec.rb +++ b/spec/mixlib/shellout/packaging_spec.rb @@ -58,7 +58,7 @@ def load_gemspec(name) it "adds the win32 dependencies used by lib/mixlib/shellout/windows.rb" do expect(spec.runtime_dependencies.map(&:name)) - .to contain_exactly("chef-utils", "win32-process", "wmi-lite", "ffi-win32-extensions") + .to contain_exactly("chef-utils", "win32-process", "wmi-lite", "win32ole", "ffi-win32-extensions") end it "ships the same files as the ruby platform gem" do @@ -91,7 +91,10 @@ def ruby_in_clean_process(*flags, code) end it "loads and runs a command with frozen string literals and warnings enabled" do - out = ruby_in_clean_process("-W:deprecated", "-w", "--enable-frozen-string-literal", <<~RUBY) + # windows/core_ext.rb deliberately redefines win32-process's Process.create, + # which -w reports, so only check frozen string literals there. + flags = windows? ? [] : ["-W:deprecated", "-w"] + out = ruby_in_clean_process(*flags, "--enable-frozen-string-literal", <<~RUBY) require "mixlib/shellout" require "mixlib/shellout/helper" cmd = Mixlib::ShellOut.new(#{RbConfig.ruby.dump}, "-e", "print :ok") From 0b7c17d03d13c03220250c4a4662e455a71ba876 Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Sat, 26 Sep 2026 00:13:54 -0700 Subject: [PATCH 7/7] Fix specs that assumed macOS behavior or unverified doubles - Run the killed-by-signal spec without a shell. dash, /bin/sh on Debian/Ubuntu, forks instead of exec'ing, so it survived and reported 137 rather than being the killed process. - Use real doubles in the kill_process_tree specs; stubbing methods that bare Objects don't have fails with verified partial doubles. - Skip -w in the load spec on Windows, where core_ext.rb deliberately redefines win32-process's Process.create. Signed-off-by: Tim Smith --- spec/mixlib/shellout/windows_spec.rb | 8 ++++---- spec/mixlib/shellout_spec.rb | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/spec/mixlib/shellout/windows_spec.rb b/spec/mixlib/shellout/windows_spec.rb index 5bc52c7..8f67524 100644 --- a/spec/mixlib/shellout/windows_spec.rb +++ b/spec/mixlib/shellout/windows_spec.rb @@ -113,10 +113,10 @@ def self.with_command(_command, &example) describe ".kill_process_tree" do let(:shell_out) { Mixlib::ShellOut.new } - let(:wmi) { Object.new } - let(:wmi_ole_object) { Object.new } - let(:wmi_process) { Object.new } - let(:logger) { Object.new } + let(:wmi) { double("wmi") } + let(:wmi_ole_object) { double("wmi_ole_object") } + let(:wmi_process) { double("wmi_process") } + let(:logger) { double("logger") } before do allow(wmi).to receive(:query).and_return([wmi_process]) diff --git a/spec/mixlib/shellout_spec.rb b/spec/mixlib/shellout_spec.rb index 94d292d..6e81cf9 100644 --- a/spec/mixlib/shellout_spec.rb +++ b/spec/mixlib/shellout_spec.rb @@ -1603,7 +1603,9 @@ def ruby_wo_shell(code) end context "when the child is killed by a signal", :unix_only do - let(:ruby_code) { "Process.kill(:KILL, Process.pid)" } + # No shell: dash (/bin/sh on Debian/Ubuntu) forks rather than execs, so + # it would survive and report 128+9 instead of being the killed process. + let(:shell_cmd) { Mixlib::ShellOut.new(RbConfig.ruby, "-e", "Process.kill(:KILL, Process.pid)") } it "has no exit status" do expect(executed_cmd.exitstatus).to be_nil