diff --git a/.expeditor/config.yml b/.expeditor/config.yml index 5a4cd545..da8335b1 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/.expeditor/run_linux_tests.sh b/.expeditor/run_linux_tests.sh deleted file mode 100755 index 19896880..00000000 --- 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 d2f4cf45..00000000 --- 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 7edb5718..00000000 --- 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/.github/copilot-instructions.md b/.github/copilot-instructions.md index 9493aee4..6ae01925 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/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000..ef8b519f --- /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 c51c3d64..73f78147 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 00000000..8495f2fe --- /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 0ae9f32b..0d8fd4c0 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 90ae24c2..4ec301ee 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/.gitignore b/.gitignore index 5064e3c5..f6a7f719 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,4 @@ Gemfile.lock */tags *~ vendor/ +spec/examples.txt diff --git a/.rspec b/.rspec index 42b555a2..7a2cc1a6 100644 --- a/.rspec +++ b/.rspec @@ -1 +1,3 @@ --f documentation --color +--require spec_helper +--format documentation +--color diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 03fdbfa7..a0bbb962 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/Gemfile b/Gemfile index 1d1fbbbe..245f538f 100644 --- a/Gemfile +++ b/Gemfile @@ -3,13 +3,10 @@ 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" +# 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 @@ -20,12 +17,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/README.md b/README.md index 4471d9c9..3c244488 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, diff --git a/cspell.json b/cspell.json index e52f6db4..8acd354e 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 ff45bd59..8a817e2a 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? diff --git a/lib/mixlib/shellout/unix.rb b/lib/mixlib/shellout/unix.rb index 3010da29..1e3cd2dd 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 diff --git a/mixlib-shellout-universal-mingw-ucrt.gemspec b/mixlib-shellout-universal-mingw-ucrt.gemspec index 3972f3ae..3aac3a37 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/mixlib-shellout.gemspec b/mixlib-shellout.gemspec index d5544373..4c9275da 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" diff --git a/spec/mixlib/shellout/helper_spec.rb b/spec/mixlib/shellout/helper_spec.rb index 8c1136df..85436d51 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 00000000..e5607c23 --- /dev/null +++ b/spec/mixlib/shellout/packaging_spec.rb @@ -0,0 +1,107 @@ +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", "win32ole", "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 + # 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") + 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 2b273a6c..8f67524e 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 @@ -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 08a12f17..6e81cf95 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,103 @@ 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 + # 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 + 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 +1685,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 420d9f41..ea5b401c 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 f4f1af81..00000000 --- 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 00000000..502cca15 --- /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)