From 57f712546b6ae2d840bc3bc92c14842754380dc9 Mon Sep 17 00:00:00 2001
From: Luchian Nemes <luchian.nemes@puppet.com>
Date: Wed, 4 Nov 2020 12:21:05 +0200
Subject: [PATCH] (PUP-10627) Add default file permissions 640 for
 last_run_summary.yaml

Due to security concerns, this commit downgrades file permissions for
`last_run_summary.yaml` from `644` to `640`. This allows only file owner
and its group access to it by default.
---
 .../tests/agent/last_run_summary_report.rb    | 22 ++++++++++++++-----
 lib/puppet/defaults.rb                        |  2 +-
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/acceptance/tests/agent/last_run_summary_report.rb b/acceptance/tests/agent/last_run_summary_report.rb
index 9852fac71a9..0fd51758ab7 100644
--- a/acceptance/tests/agent/last_run_summary_report.rb
+++ b/acceptance/tests/agent/last_run_summary_report.rb
@@ -50,7 +50,7 @@
       end
     end
 
-    step "Check if the 'last_run_summary.yaml' report file created has '0644' permissions" do
+    step "Check if the 'last_run_summary.yaml' report file created has '0640' permissions" do
       if agent['platform'] =~ /windows/
         on(agent, "icacls #{File.join(publicdir, 'last_run_summary.yaml')}") do |result|
           # Linux 'Owner' premissions class equivalent
@@ -58,11 +58,16 @@
           # Linux 'Group' permissions class equivalent
           assert_match('None:(R)', result.stdout)
           # Linux 'Public' permissions class equivalent
-          assert_match('Everyone:(R)', result.stdout)
+          assert_match('Everyone:(Rc,S,RA)', result.stdout)
+          # According to icacls docs:
+          # Rc = Read control
+          # S  = Synchronize
+          # RA = Read attributes
+          # More at https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/icacls
         end
       else
         on(agent, "ls -al #{publicdir}") do |result|
-          assert_match(/rw-r--r--.+last_run_summary\.yaml$/, result.stdout)
+          assert_match(/rw-r-----.+last_run_summary\.yaml$/, result.stdout)
         end
       end
     end
@@ -86,7 +91,7 @@
       end
     end
 
-    step "Check if the 'last_run_summary.yaml' report file was created in the new location and still has '0644' permissions" do
+    step "Check if the 'last_run_summary.yaml' report file was created in the new location and still has '0640' permissions" do
       if agent['platform'] =~ /windows/
         on(agent, "icacls #{File.join(custom_publicdir, 'last_run_summary.yaml')}") do |result|
           # Linux 'Owner' premissions class equivalent
@@ -94,11 +99,16 @@
           # Linux 'Group' permissions class equivalent
           assert_match('None:(R)', result.stdout)
           # Linux 'Public' permissions class equivalent
-          assert_match('Everyone:(R)', result.stdout)
+          assert_match('Everyone:(Rc,S,RA)', result.stdout)
+          # According to icacls docs:
+          # Rc = Read control
+          # S  = Synchronize
+          # RA = Read attributes
+          # More at https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/icacls
         end
       else
         on(agent, "ls -al #{custom_publicdir}") do |result|
-          assert_match(/rw-r--r--.+last_run_summary\.yaml$/, result.stdout)
+          assert_match(/rw-r-----.+last_run_summary\.yaml$/, result.stdout)
         end
       end
     end
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 91abb1e7674..1f2d6ddbc11 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -1809,7 +1809,7 @@ def self.initialize_default_settings!(settings)
     :lastrunfile =>  {
       :default  => "$publicdir/last_run_summary.yaml",
       :type     => :file,
-      :mode     => "0644",
+      :mode     => "0640",
       :desc     => "Where puppet agent stores the last run report summary in yaml format."
     },
     :lastrunreport =>  {