Security fix, enforce file ownership, and run acceptance test on macOS

.github/workflows/pr-checks.yaml

@@ -54,30 +54,42 @@ jobs:
     name: Acceptance tests
     strategy:
       matrix:
-        os:
-          - ubuntu-latest
-        agent:
-          - puppet7
-          - puppet8
+        include:
+          - os: ubuntu-latest
+            puppet: 6
+          - os: ubuntu-latest
+            puppet: 7
+          - os: ubuntu-latest
+            puppet: 8
+          - os: macos-latest
+            puppet: 8
     runs-on: ${{ matrix.os }}
 
     steps:
     - uses: actions/checkout@v4
     - name: Install Puppet
       run: |
         set -ex
-        distro=$(lsb_release -cs)
-        deb_name="${{ matrix.agent }}-release-${distro}.deb"
-        curl -sSO  "https://apt.puppet.com/${deb_name}"
-        sudo dpkg -i "$deb_name"
-        rm "$deb_name"
-        sudo apt-get update -qq
-        sudo apt-get install -qy puppet-agent pdk
-    - name: Build module
-      run: pdk build
-    - name: Install module
-      run: sudo -E /opt/puppetlabs/bin/puppet module install pkg/*.tar.gz
+        case ${{ matrix.os }} in
+          macos*)
+            brew install --cask puppetlabs/puppet/puppet-agent-${{ matrix.puppet }}
+            brew install --cask puppetlabs/puppet/pdk
+          ;;
+          ubuntu*)
+            distro=$(lsb_release -cs)
+            deb_name="puppet${{ matrix.puppet }}-release-${distro}.deb"
+            curl -sSO  "https://apt.puppet.com/${deb_name}"
+            sudo dpkg -i "$deb_name"
+            rm "$deb_name"
+            sudo apt-get update -qq
+            sudo apt-get install -qy puppet-agent pdk
+          ;;
+          *)
+            echo ::error::Unsupported platform
+            exit 1
+          ;;
+        esac
     - name: Install PDK dependencies
-      run: sudo -E pdk bundle install
+      run: sudo -E /opt/puppetlabs/pdk/bin/pdk bundle install
     - name: Run acceptance tests
-      run: sudo -E pdk bundle exec rake litmus:acceptance:localhost
+      run: sudo -E /opt/puppetlabs/pdk/bin/pdk bundle exec rake litmus:acceptance:localhost

.sync.yml

@@ -6,6 +6,9 @@
 # See https://github.com/danielparks/pdk-templates/blob/main/config_defaults.yml
 # for the default values.
 ---
+.github/workflows/pr-checks.yaml:
+  additional-platforms:
+    - macos-latest
 spec/default_facts.yml:
   extra_facts:
     identity:

CHANGELOG.md

@@ -4,6 +4,45 @@ All notable changes to this project will be documented in this file.
 
 ## main branch
 
+### Security fix
+
+Certain Go tarballs (see below) had files owned by non-root users:
+
+    ❯ curl -SsL https://go.dev/dl/go1.20.14.darwin-amd64.tar.gz | tar -tzvf - | head -3
+    drwxr-xr-x  0 0      0           0 Feb  2 10:19 go/
+    -rw-r--r--  0 gopher wheel    1339 Feb  2 10:09 go/CONTRIBUTING.md
+    -rw-r--r--  0 gopher wheel    1479 Feb  2 10:09 go/LICENSE
+
+In this case, the non-root user in question mapped to the first user created on
+the macOS system (UID 501).
+
+When running as root, previous versions of dp-golang would preserve file
+ownership when extracting the tarball, even if `owner` was set to something
+else. **This meant that files, such as the `go` binary, ended up being writable
+by a non-root user.**
+
+This version of dp-golang enables [`tar`]’s `--no-same-owner` and
+`--no-same-permissions` flags, which cause files to be extracted as the user
+running Puppet, or as the user/group specified in the Puppet code.
+
+GitHub security advisory: [GHSA-8h8m-h98f-vv84]
+
+#### Affected Go tarballs
+
+  * Go for macOS version 1.4.3 through 1.21rc3, inclusive.
+  * go1.4-bootstrap-20170518.tar.gz
+  * go1.4-bootstrap-20170531.tar.gz
+
+[`tar`]: https://www.man7.org/linux/man-pages/man1/tar.1.html
+[GHSA-8h8m-h98f-vv84]: https://github.com/danielparks/puppet-golang/security/advisories/GHSA-8h8m-h98f-vv84
+
+### Changes
+
+As part of the security fix mentioned above, it became necessary to be more
+agressive about ensuring that the owner and group of files in the installation
+are correct. dp-golang now deletes and recreates any Go installation it finds
+that has a file or directory with the wrong owner or group.
+
 ## Release 1.2.6
 
 * Synced with [PDK][].

manifests/from_tarball.pp

@@ -46,6 +46,9 @@
   String[1]                          $mode       = '0755',
   Stdlib::Unixpath                   $state_file = golang::state_file($go_dir),
 ) {
+  $encoded_go_dir = $go_dir.regsubst('/', '_', 'G')
+  $archive_path = "/tmp/puppet-golang${encoded_go_dir}.tar.gz"
+
   if $ensure != any_version {
     # Used to ensure that the installation is updated when $source changes.
     $file_ensure = $ensure ? {
@@ -70,6 +73,38 @@
     }
   }
 
+  if $ensure == present or $ensure == any_version {
+    # Remove Go installation if any of its files have the wrong user or group.
+    # This will cause it to be replaced with a fresh installation.
+    exec { "dp/golang check ownership of ${go_dir}":
+      command     => ['rm', '-rf', $go_dir],
+      environment => [
+        "GO_DIR=${go_dir}",
+        "OWNER=${owner}",
+        "GROUP=${group}",
+      ],
+      path        => ['/usr/local/bin', '/usr/bin', '/bin'],
+      onlyif      => 'find "$GO_DIR" "(" "(" -not -user "$OWNER" ")" -or "(" -not -group "$GROUP" ")" ")" -print -quit | grep .',
+      before      => File[$go_dir],
+      notify      => Archive[$archive_path],
+    }
+  }
+
+  # File[$state_file] changing should only trigger an update when ensure is
+  # present, and not any_version.
+  if $ensure == present {
+    # If the $go_dir/bin directory exists, archive won't update it. Also, we
+    # want to remove any files that are not present in the new version.
+    exec { "dp/golang refresh go installation at ${go_dir}":
+      command     => ['rm', '-rf', $go_dir],
+      path        => ['/usr/local/bin', '/usr/bin', '/bin'],
+      refreshonly => true,
+      subscribe   => File[$state_file],
+      before      => File[$go_dir],
+      notify      => Archive[$archive_path],
+    }
+  }
+
   $directory_ensure = $ensure ? {
     'present'     => directory,
     'any_version' => directory,
@@ -85,31 +120,13 @@
   }
 
   if $ensure == present or $ensure == any_version {
-    $encoded_go_dir = $go_dir.regsubst('/', '_', 'G')
-    $archive_path = "/tmp/puppet-golang${encoded_go_dir}.tar.gz"
-
-    # Only trigger an update when ensure is present, and not any_version.
-    if $ensure == present {
-      # If the $go_dir/bin directory exists, archive won't update it. Also, we
-      # want to remove any files that are not present in the new version.
-      exec { "dp/golang refresh go installation at ${go_dir}":
-        command     => ['rm', '-rf', $go_dir],
-        path        => ['/usr/local/bin', '/usr/bin', '/bin'],
-        user        => $facts['identity']['user'],
-        refreshonly => true,
-        subscribe   => File[$state_file],
-        before      => File[$go_dir],
-        notify      => Archive[$archive_path],
-      }
-    }
-
     include archive
 
     archive { $archive_path:
       ensure        => present,
       extract       => true,
       extract_path  => $go_dir,
-      extract_flags => '--strip-components 1 -xf',
+      extract_flags => '--strip-components 1 --no-same-owner --no-same-permissions -xf',
       user          => $owner,
       group         => $group,
       source        => $source,

metadata.json

@@ -85,5 +85,5 @@
   ],
   "pdk-version": "3.0.1",
   "template-url": "https://github.com/danielparks/pdk-templates#main",
-  "template-ref": "heads/main-0-g2f62871"
+  "template-ref": "heads/main-0-gde8efe4"
 }