Merge pull request #773 from joseivanlopez/root_warning

Improving check-profile

src/lib/autoinstall/clients/autoyast.rb

@@ -1,4 +1,4 @@
-# Copyright (c) [2020] SUSE LLC
+# Copyright (c) [2020-2021] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -102,24 +102,45 @@ def main
             }
           },
           "options"    => {
-            "filename"    => { "type" => "string", "help" => "Which profile to use. In " \
-              "check-profile case it supports also remote location like " \
-              "filename=ftp://test.com/example.xml" },
-            "modname"     => { "type" => "string", "help" => "modname=AYAST_MODULE" },
-            "output"      => { "type" => "string", "help" => "where evaluated profile will be " \
-              "written. Default is '~/check_profile_result.xml'. Example 'filename=~/test.xml'." },
-            "run-scripts" => { "type"     => "enum",
-                               "help"     => "run also scripts that are defined in profile. " \
-              "By default false. Example: run-scripts=true",
-                               "typespec" => ["true", "false"] },
-            "import-all"  => { "type"     => "enum",
-                               "help"     => "Do testing import of all sections in profile. " \
-              "Note that scripts are imported when run-scripts is set to true. By default true. " \
-              "Example: import-all=false",
-                               "typespec" => ["true", "false"] }
+            "filename"    => {
+              "type" => "string",
+              "help" => "Which profile to use. In check-profile case it supports also remote " \
+                        "location like filename=ftp://test.com/example.xml"
+            },
+            "modname"     => {
+              "type" => "string",
+              "help" => "modname=AYAST_MODULE"
+            },
+            "output"      => {
+              "type" => "string",
+              "help" => "Where evaluated profile will be written. Default is " \
+                        "'~/check_profile_result.xml'. Example 'filename=~/test.xml'."
+            },
+            "run-scripts" => {
+              "type"     => "enum",
+              "help"     => "Run also scripts that are defined in profile. Be careful when " \
+                            "running pre-installation scripts as root. Use only\n\t\t\t\t     " \
+                            "profiles that you trust. By default false. Example: run-scripts=true",
+              "typespec" => ["true", "false"]
+            },
+            "run-erb"     => {
+              "type"     => "enum",
+              "help"     => "Render the ERB profile. Be careful when running ERB profiles as " \
+                            "root. Use only profiles that you trust. This option is" \
+                            "\n\t\t\t\t     mandatory when running checks for an ERB profile as " \
+                            "root. Example: run-erb=true",
+              "typespec" => ["true", "false"]
+            },
+            "import-all"  => {
+              "type"     => "enum",
+              "help"     => "Do testing import of all sections in profile. Note that scripts are " \
+                            "imported when run-scripts is set to true.\n\t\t\t\t     By default " \
+                            "true. Example: import-all=false",
+              "typespec" => ["true", "false"]
+            }
           },
           "mappings"   => {
-            "check-profile" => ["filename", "run-scripts", "output", "import-all"],
+            "check-profile" => ["filename", "run-scripts", "run-erb", "output", "import-all"],
             "file"          => ["filename", "modname"],
             "module"        => ["filename", "modname"],
             "ui"            => ["filename", "modname"]
@@ -171,8 +192,10 @@ def check_profile(options)
           return false
         end
 
+        return false unless erb_check(options["filename"], options["run-erb"])
+
         checker = ProfileChecker.new(options["filename"],
-          import_all:  options["import_all"] != "false",
+          import_all:  options["import-all"] != "false",
           run_scripts: options["run-scripts"] == "true",
           target_file: options["output"] || "~/check_profile_result.xml")
         checker.check
@@ -201,6 +224,34 @@ def import_profile(filename)
         Yast::Popup.ClearFeedback
       end
 
+      # Checks whether an ERB profile can be rendered according to the given options
+      #
+      # An ERB can be rendered if the check-profile command is run without root permissions. If it
+      # is run as root, then the option run-erb=true must be given.
+      #
+      # @param filename [String] filename of the AutoYaST profile
+      # @param run_erb_option [String] the given run-erb option
+      #
+      # @return [Boolean]
+      def erb_check(filename, run_erb_option)
+        return true unless filename.end_with?(".erb")
+
+        if !run_erb_option && Process.euid.zero?
+          Yast::CommandLine.Error(_("run-erb=true option is mandatory when checking an ERB " \
+            "profile as root."))
+
+          return false
+        end
+
+        if run_erb_option == "false"
+          Yast::CommandLine.Error(_("The ERB profile cannot be rendered with run-erb=false."))
+
+          return false
+        end
+
+        true
+      end
+
       # AutoYaST UI sequence
       #
       # @return [Y2Autoinstallation::AutoSequence]
@@ -264,7 +315,9 @@ def list_modules_action_help
       def check_profile_action_help
         _("Check if profile is valid. Also evaluate profile for dynamic profiles like " \
           "ERB or rules/classes. If run-scripts parameter is set to 'true' it can be used " \
-          "also to validate dynamic profiles generated by pre-scripts.")
+          "also to validate dynamic profiles generated by pre-scripts.\n\n    " \
+          "Be careful when running pre-installation scripts and ERB profiles as root. Use " \
+          "only profiles that you trust.")
       end
     end
   end

src/modules/ProfileLocation.rb

@@ -1,3 +1,22 @@
+# Copyright (c) [2013-2021] SUSE LLC
+#
+# All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or modify it
+# under the terms of version 2 of the GNU General Public License as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, contact SUSE LLC.
+#
+# To contact SUSE LLC about this file by physical or electronic mail, you may
+# find current contact information at www.suse.com.
+
 # File:  modules/ProfileLocation.ycp
 # Package:  Auto-installation
 # Summary:  Process Auto-Installation Location
@@ -11,6 +30,7 @@
 require "autoinstall/y2erb"
 require "y2storage"
 require "fileutils"
+require "yast2/popup"
 
 module Yast
   class ProfileLocationClass < Module
@@ -154,8 +174,7 @@ def Process
 
         # render erb template
         if AutoinstConfig.filepath.end_with?(".erb")
-          res = Y2Autoinstallation::Y2ERB.render(localfile)
-          SCR.Write(path(".target.string"), localfile, res)
+          return false unless render_erb(localfile)
         end
       else
         is_directory = true
@@ -257,6 +276,30 @@ def Process
 
     publish function: :ProfileLocation, type: "void ()"
     publish function: :Process, type: "boolean ()"
+
+  private
+
+    # Renders the ERB profile and saves the result
+    #
+    # An error popup is shown if there is an error while rendering the profile.
+    #
+    # @return [Boolean] true if everything was ok.
+    def render_erb(file)
+      res = nil
+
+      begin
+        res = Y2Autoinstallation::Y2ERB.render(file)
+      rescue StandardError => e
+        message = _("There was an error while rendering the ERB profile.")
+        details = e.message + "\n\n" + e.backtrace.join("\n")
+
+        Yast2::Popup.show(message, headline: :error, details: details)
+        return false
+      end
+
+      SCR.Write(path(".target.string"), file, res)
+      true
+    end
   end
 
   ProfileLocation = ProfileLocationClass.new

test/ProfileLocation_test.rb

@@ -1,5 +1,24 @@
 #!/usr/bin/env rspec
 
+# Copyright (c) [2019-2021] SUSE LLC
+#
+# All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or modify it
+# under the terms of version 2 of the GNU General Public License as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, contact SUSE LLC.
+#
+# To contact SUSE LLC about this file by physical or electronic mail, you may
+# find current contact information at www.suse.com.
+
 require_relative "test_helper"
 
 Yast.import "ProfileLocation"
@@ -9,18 +28,19 @@
   subject { Yast::ProfileLocation }
 
   describe "#Process" do
-
     before do
       Yast::AutoinstConfig.scheme = "relurl"
       Yast::AutoinstConfig.xml_tmpfile = "/tmp/123"
-      Yast::AutoinstConfig.filepath = "autoinst.xml"
+      Yast::AutoinstConfig.filepath = filepath
       allow(Yast::InstURL).to receive(:installInf2Url).and_return(
         "http://download.opensuse.org/distribution/leap/15.1/repo/oss/"
       )
       allow(Yast::SCR).to receive(:Read).and_return("test")
       allow(Yast::Report).to receive(:Error) # test is already quite weak and some errors are shown
     end
 
+    let(:filepath) { "autoinst.xml" }
+
     context "when scheme is \"relurl\"" do
       it "downloads AutoYaST configuration file with absolute path" do
         expect(subject).to receive(:Get).with("http",
@@ -50,5 +70,51 @@
         subject.Process
       end
     end
+
+    context "when the profile is an erb file" do
+      let(:filepath) { "autoinst.erb" }
+
+      before do
+        allow(Yast2::Popup).to receive(:show)
+
+        allow(subject).to receive(:Get).and_return("test")
+
+        allow(Yast::GPG).to receive(:encrypted_symmetric?).and_return(false)
+      end
+
+      context "and there is no error rendering the erb profile" do
+        before do
+          allow(Y2Autoinstallation::Y2ERB).to receive(:render)
+            .with(Yast::AutoinstConfig.xml_tmpfile).and_return("rendered content")
+
+          allow(Y2Autoinstallation::XmlChecks.instance).to receive(:valid_profile?).and_return(true)
+
+          allow(Yast::SCR).to receive(:Write)
+        end
+
+        it "does not show rendering errors" do
+          expect(Yast2::Popup).to_not receive(:show)
+
+          subject.Process
+        end
+      end
+
+      context "and there is some error rendering the erb profile" do
+        before do
+          allow(Y2Autoinstallation::Y2ERB).to receive(:render)
+            .with(Yast::AutoinstConfig.xml_tmpfile).and_raise StandardError
+        end
+
+        it "shows a rendering error" do
+          expect(Yast2::Popup).to receive(:show).with(/error while rendering/, anything)
+
+          subject.Process
+        end
+
+        it "returns false" do
+          expect(subject.Process).to eq(false)
+        end
+      end
+    end
   end
 end