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.

Author: danielparks

CHANGELOG.md

@@ -26,11 +26,15 @@ 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.
 
-**This will not fix existing installs** unless you update the Go version, or
-otherwise prompt Puppet to reinstall.
-
 [`tar`]: https://www.man7.org/linux/man-pages/man1/tar.1.html
 
+### 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. This 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

@@ -187,6 +187,46 @@
       end
     end
 
+    context 'after chowning bin/go' do
+      File.chown(0, 0, "#{home}/user/go-install/bin/go")
+
+      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
+
     context 'cleans up' do
       it 'uninstalls Go' do
         idempotent_apply(<<~"PUPPET")