Jira (PUP-5915) exec resources fail unless cwd is readable

12 views
Skip to first unread message

James Ralston (JIRA)

unread,
Feb 17, 2016, 1:45:03 PM2/17/16
to puppe...@googlegroups.com
James Ralston created an issue
 
Puppet / Bug PUP-5915
exec resources fail unless cwd is readable
Issue Type: Bug Bug
Affects Versions: PUP 4.3.2
Assignee: Unassigned
Created: 2016/02/17 10:44 AM
Priority: Normal Normal
Reporter: James Ralston

The exec() resource always attempts to perform a chdir() to the current working directory (cwd). This is true regardless of the value of the cwd parameter.

If the cwd isn't accessible by the user running Puppet, Puppet will fail to apply the exec() resource. This is true regardless of whether the command being executed actually requires the cwd to be readable.

An example:

$ cd /tmp
$ mkdir test
$ cd test
$ chmod 0000 .
 
$ puppet apply -e 'exec { "/bin/pwd": logoutput => true }'
Notice: Compiled catalog for myhost.example.org in environment production in 0.05 seconds
Error: Permission denied @ dir_chdir - /tmp/test
Error: /Stage[main]/Main/Exec[/bin/pwd]/returns: change from notrun to 0 failed: Permission denied @ dir_chdir - /tmp/test
Notice: Applied catalog in 0.04 seconds
 
$ puppet apply -e 'exec { "/bin/pwd": logoutput => true, cwd => "/" }'
Notice: Compiled catalog for myhost.example.org in environment production in 0.06 seconds
Error: Permission denied @ dir_chdir - /tmp/test
Error: /Stage[main]/Main/Exec[/bin/pwd]/returns: change from notrun to 0 failed: Permission denied @ dir_chdir - /tmp/test
Notice: Applied catalog in 0.05 seconds

If the cwd is accessible, the exec will succeed:

$ chmod 0755 .
 
$ puppet apply -e 'exec { "/bin/pwd": logoutput => true }'
Notice: Compiled catalog for myhost.example.org in environment production in 0.05 seconds
Notice: /Stage[main]/Main/Exec[/bin/pwd]/returns: /tmp/test
Notice: /Stage[main]/Main/Exec[/bin/pwd]/returns: executed successfully
Notice: Applied catalog in 0.05 seconds

You may be tempted to respond: "Why does this matter? It's almost never useful to run Puppet as a non-root user, and the root user has implicit access to all files and directories."

The problem is that this is not true.

At our site, we are deploying NFSv4 user home directories, where home directories are mounted from an NFSv4 server with sec=krb5p (that is, with Kerberos security). In such a scenario, the root user always and implicit possesses host/hostname@DOMAIN credentials, and nothing more.

User home directories are private (mode 0700) by default. Effectively, this means that the root user is unable to chdir() to a user's home directory, or any subdirectory thereof.

As we began to deploy NFSv4 user home directories, developers who run sudo puppet agent --test from the command line began reporting Puppet failures. Investigating the problems let us to discover the exec resource's behavior of always attempting to chdir() to the cwd, regardless of what the cwd parameter is set to (or whether it's defined at all).

Our developers get hit by this because they are typically sitting in their (NFSv4-based) home directory when they attempt to run sudo puppet agent --test.

I would argue that the exec resource should behave this way:

  1. If the cwd parameter is defined, the exec resource attempts to chdir() to that directory, and only that directory.
  2. If the cwd parameter is undefined, the exec resource should not call chdir() at all.

The current behavior of the exec resource is at best extremely non-intuitive and at worst outright incorrect, even if it only causes problems when specific (and uncommon) conditions are met (i.e., root not having implicit access to the cwd).

Thoughts?

Add Comment Add Comment
 
This message was sent by Atlassian JIRA (v6.4.12#64027-sha1:e3691cc)
Atlassian logo

James Ralston (JIRA)

unread,
Feb 17, 2016, 2:14:04 PM2/17/16
to puppe...@googlegroups.com
James Ralston commented on Bug PUP-5915
 
Re: exec resources fail unless cwd is readable

Also, unless I'm missing something, the fix is pretty simple. Here's a quick diff (ignoring whitespace changes, so that it's easier to read):

--- /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/provider/exec.rb.ORIG   2016-02-02 14:10:21.000000000 -0500
+++ /opt/puppetlabs/puppet/lib/ruby/vendor_ruby/puppet/provider/exec.rb 2016-02-17 14:05:12.750781461 -0500
@@ -12,7 +12,9 @@
     checkexe(command)
 
     if dir = resource[:cwd]
-      unless File.directory?(dir)
+      if File.directory?(dir)
+        Dir.chdir(dir)
+      else
         if check
           dir = nil
         else
@@ -21,12 +23,8 @@
       end
     end
 
-    dir ||= Dir.pwd
-
     debug "Executing#{check ? " check": ""} '#{command}'"
     begin
-      # Do our chdir
-      Dir.chdir(dir) do
         environment = {}
 
         environment[:PATH] = resource[:path].join(File::PATH_SEPARATOR) if resource[:path]
@@ -66,7 +64,6 @@
           raise ArgumentError, output
         end
 
-      end
     rescue Errno::ENOENT => detail
       self.fail Puppet::Error, detail.to_s, detail
     end

I've tested this locally, and it seems to work.

This does move the chdir() call outside of the begin/rescue/end block, though. Does the chdir() call by itself warrant its own begin/rescue/end block?

Anyway, if this looks good, I'll be happy to submit a merge request.

Henrik Lindberg (JIRA)

unread,
May 15, 2017, 2:58:05 PM5/15/17
to puppe...@googlegroups.com

James Ralston did you get around to making a PR?

This message was sent by Atlassian JIRA (v6.4.14#64029-sha1:ae256fe)
Atlassian logo

Henrik Lindberg (JIRA)

unread,
May 15, 2017, 2:58:05 PM5/15/17
to puppe...@googlegroups.com

Geoff Nichols (JIRA)

unread,
May 25, 2017, 1:22:08 PM5/25/17
to puppe...@googlegroups.com

Doug Penner (JIRA)

unread,
Jan 29, 2018, 11:35:03 PM1/29/18
to puppe...@googlegroups.com
Doug Penner commented on Bug PUP-5915
 
Re: exec resources fail unless cwd is readable

I can confirm that the problem also exists in puppet 5.3.3.

If we are in our home folders on a system with root-squash, we get the same error as James whether we set a cwd or not.

This message was sent by Atlassian JIRA (v7.0.2#70111-sha1:88534db)
Atlassian logo

James Ralston (JIRA)

unread,
Mar 2, 2018, 3:57:04 PM3/2/18
to puppe...@googlegroups.com
James Ralston commented on Bug PUP-5915

My first attempt at solving the problem was incorrect, which is why I never submitted a PR. But this is still biting us, so I'll take another crack at it.

This message was sent by Atlassian JIRA (v7.7.1#77002-sha1:e75ca93)
Atlassian logo

Adam Winberg (JIRA)

unread,
Aug 14, 2018, 6:43:02 AM8/14/18
to puppe...@googlegroups.com

James Ralston (JIRA)

unread,
Oct 8, 2019, 4:34:03 PM10/8/19
to puppe...@googlegroups.com
James Ralston commented on Bug PUP-5915

I was incorrect. This is not actually fixed.

The problem is that the exec resource itself implicitly defaults the cwd parameter to the cwd if the cwd parameter was not provided for the exec resource in question.

In the case where Puppet's cwd is inaccessible, this will cause all Puppet exec() resources to fail.

I assert that defaulting the cwd parameter to the cwd of the Puppet process is wrong. If the user does not supply a cwd parameter to the exec resource in question, Puppet must assume that the user does not want Puppet to attempt to chdir() to any directory. Assuming that the user wants Puppet to chdir() to the cwd is an erroneous assumption, as 1) this is not what the user specified, and 2) it's nonsensical, as even if the chdir() call succeeds, Puppet has not actually changed the cwd.

To fully fix this bug, I agree with John Bollinger in PUP-10080: the "implicitly default the cwd parameter to Puppet's cwd if it was not specified" behavior is wrong and should be removed.

I think the only thing required is removing this line from lib/puppet/provider/exec.rb:

      cwd ||= Dir.pwd

This is because, from my read, Puppet::Util::Execution.execute() will do the correct thing if the cwd parameter is unspecified:

    cwd = options[:cwd]
    if cwd && ! Puppet::FileSystem.directory?(cwd)
      raise ArgumentError, _("Working directory %{cwd} does not exist!") % { cwd: cwd }
    end

And:

        cwd = options[:cwd]
        Dir.chdir(cwd) if cwd

Josh Cooper (JIRA)

unread,
Oct 8, 2019, 5:56:04 PM10/8/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
 
Change By: Josh Cooper
Fix Version/s: PUP 5.5.7
Fix Version/s: PUP 6.11.0
Fix Version/s: PUP 6.4.5
Fix Version/s: PUP 5.5.18

Josh Cooper (JIRA)

unread,
Oct 8, 2019, 5:56:04 PM10/8/19
to puppe...@googlegroups.com
Josh Cooper assigned an issue to Josh Cooper
Change By: Josh Cooper
Assignee: Josh Cooper

Josh Cooper (JIRA)

unread,
Oct 8, 2019, 5:57:03 PM10/8/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Sprint: Platform Core KANBAN

Josh Cooper (JIRA)

unread,
Oct 15, 2019, 12:40:04 PM10/15/19
to puppe...@googlegroups.com

Josh Cooper (JIRA)

unread,
Oct 15, 2019, 12:51:04 PM10/15/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Release Notes Summary: Previously exec resources now only change the current working directory if the `cwd` parameter is specified. Previously if the `cwd` parameter was not specified, puppet would change its working directory to the current working directory, which was redundant and could fail if the current working directory was not accessible.
Release Notes: Bug Fix

Josh Cooper (JIRA)

unread,
Oct 21, 2019, 1:08:04 PM10/21/19
to puppe...@googlegroups.com

Heston Hoffman (JIRA)

unread,
Nov 18, 2019, 1:29:05 PM11/18/19
to puppe...@googlegroups.com
Heston Hoffman updated an issue
 
Change By: Heston Hoffman
Labels: resolved-issue-added
Reply all
Reply to author
Forward
0 new messages