Enforce owner/group of Go installations

This now recreates a Go installation if any of its files or directories
have the wrong owner or group.

CHANGELOG.md

@@ -36,6 +36,13 @@ GitHub security advisory: [GHSA-8h8m-h98f-vv84]
 [`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,24 +120,6 @@
   }
 
   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:

spec/acceptance/golang_from_tarball_spec.rb

@@ -185,6 +185,48 @@
         its(:owner) { is_expected.to eq 'user' }
         its(:group) { is_expected.to eq 'user' }
       end
+
+      context 'enforcing file ownership' do
+        it 'chowns bin/go' do
+          File.chown(0, 0, "#{home}/user/go-install/bin/go")
+        end
+
+        it 'reinstalls Go' do
+          apply_manifest(<<~"PUPPET", expect_changes: true)
+            golang::from_tarball { '#{home}/user/go-install':
+              source => '#{source_url}',
+              owner  => 'user',
+              group  => 'user',
+              mode   => '0700',
+            }
+          PUPPET
+        end
+
+        describe file("#{home}/user/go-install") do
+          it { is_expected.to be_directory }
+          its(:mode) { is_expected.to eq '700' }
+          its(:owner) { is_expected.to eq 'user' }
+          its(:group) { is_expected.to eq 'user' }
+        end
+
+        describe file("#{home}/user/go-install/bin/go") do
+          it { is_expected.to be_file }
+          its(:mode) { is_expected.to eq '755' }
+          its(:owner) { is_expected.to eq 'user' }
+          its(:group) { is_expected.to eq 'user' }
+        end
+
+        it 'does nothing' do
+          apply_manifest(<<~"PUPPET", catch_changes: true)
+            golang::from_tarball { '#{home}/user/go-install':
+              source => '#{source_url}',
+              owner  => 'user',
+              group  => 'user',
+              mode   => '0700',
+            }
+          PUPPET
+        end
+      end
     end
 
     context 'cleans up' do