Sanitize the environment before spawning

[ringods]

Secure coding guidelines advice to sanitize the environment before spawning child processes.

In this case, having a GEM_PATH defined in the parent process has influence on the execution of the spawned puppet process:

C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require': cannot load such file -- win32/dir (LoadError)
        from C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
        from C:/Tools/Puppet/facter/lib/facter/util/config.rb:44:in `<top (required)>'
        from C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
        from C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
        from C:/Tools/Puppet/facter/lib/facter/util/resolution.rb:7:in `<top (required)>'
        from C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
        from C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
        from C:/Tools/Puppet/facter/lib/facter/util/fact.rb:2:in `<top (required)>'
        from C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
        from C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
        from C:/Tools/Puppet/facter/lib/facter.rb:23:in `<module:Facter>'
        from C:/Tools/Puppet/facter/lib/facter.rb:19:in `<top (required)>'
        from C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
        from C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
        from C:/Tools/Puppet/puppet/lib/puppet.rb:6:in `<top (required)>'
        from C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
        from C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
        from C:/Tools/Puppet/puppet/lib/puppet/util/command_line.rb:12:in `<top (required)>'
        from C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
        from C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'
        from C:/Tools/Puppet/puppet/bin/puppet:3:in `<main>'

When unsetting the GEM_PATH, it works flawlessly:

$ irb
1.9.3-p484 :001 > require 'open3'
 => true
1.9.3-p484 :002 > result = Open3.popen3('puppet --version')
 => [#<IO:fd 6>, #<IO:fd 7>, #<IO:fd 9>, #<Thread:0x00000600062998 run>]
1.9.3-p484 :003 > result[2].read
 => "C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require': cannot load such file -- win32/dir (LoadError)\r\n\tfrom C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'\r\n\tfrom C:/Tools/Puppet/facter/lib/facter/util/config.rb:44:in `<top (required)>'\r\n\tfrom C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'\r\n\tfrom C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'\r\n\tfrom C:/Tools/Puppet/facter/lib/facter/util/resolution.rb:7:in `<top (required)>'\r\n\tfrom C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'\r\n\tfrom C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'\r\n\tfrom C:/Tools/Puppet/facter/lib/facter/util/fact.rb:2:in `<top (required)>'\r\n\tfrom C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'\r\n\tfrom C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'\r\n\tfrom C:/Tools/Puppet/facter/lib/facter.rb:23:in `<module:Facter>'\r\n\tfrom C:/Tools/Puppet/facter/lib/facter.rb:19:in `<top (required)>'\r\n\tfrom C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'\r\n\tfrom C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'\r\n\tfrom C:/Tools/Puppet/puppet/lib/puppet.rb:6:in `<top (required)>'\r\n\tfrom C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'\r\n\tfrom C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'\r\n\tfrom C:/Tools/Puppet/puppet/lib/puppet/util/command_line.rb:12:in `<top (required)>'\r\n\tfrom C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'\r\n\tfrom C:/Tools/Puppet/sys/ruby/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:36:in `require'\r\n\tfrom C:/Tools/Puppet/puppet/bin/puppet:3:in `<main>'\r\n"
1.9.3-p484 :004 > result = Open3.popen3({"GEM_PATH"=> nil}, 'puppet --version')
 => [#<IO:fd 8>, #<IO:fd 10>, #<IO:fd 12>, #<Thread:0x00000600064108 run>]
1.9.3-p484 :005 > result[2].read
 => ""
1.9.3-p484 :006 > result[1].read
 => "3.3.1\n"
1.9.3-p484 :007 >

[ringods]

The Open3.popen3 backport from open3_backport does not accept the env argument in Ruby 1.8.7. Do you guys know how to resolve this?

[carlossg]

maybe executing the command GEM_PATH="" puppet ... under ruby 1.8 ?

[njam]

Or something like?

      env_backup = ENV.to_hash
      ENV['FOO'] = 'foo'
      response = `command`
      ENV.replace(env_backup)

[ringods]

@njam your solution is inherently unsafe: you modify the ENV for the parent process too. If you are running multi-threaded, other threads might be impacted by your (temporary) change.

[ringods]

@carlossg I added a 1.8.7 code path using your suggestion, but puppet can't be found this way. I assume because we aren't actually using a system installed puppet, but the puppet gem configured as a developer dependency. As an additional note: your suggestion only works on Unix. I have to get this working on Windows too!

I have been reading up on process execution in Ruby, and to be honest, I find it a big mess: Process, Open3, Open4, IO.popen4... Every of these libraries have issues, either on a specific Ruby version, or on a specific platform. Regarding the environment, a lot of these libs don't even talk about the environment at all. Is the environment cleared completely, or taking over in full when the process is spawned?

What I'm after at the moment is a call that also works on Unix & Windows, where I can explicitly specify the environment. Any suggestion?

[carlossg]

IIUC my solution works for you in windows but not in the travis environment?
in travis the native puppet is not installed, just the gem, so as a travis pre-step you could change the PATH to include the gem puppet binary so it is available even GEM_PATH is empty

[njam]

@ringods hm makes sense. I agree on it being a big mess.
Librarian has an abstraction in posix.rb..

[ringods]

@njam The Librarian abstraction you mentioned uses the same unsafe technique in their JRuby and 1.8.x code path unfortunately.

[ringods]

The PATH wasn't an issue it seems. I added the error output from puppet --version to the output of librarian-puppet. This was the result:

https://travis-ci.org/ringods/librarian-puppet/jobs/20271651

I'm getting more and more lost...

[ringods]

@carlossg I have left the 1.8.7 situation as it is, and only sanitized the environment for >=1.9. This should fix it for the majority of the users. Will you accept my changes this way?

[carlossg]

Thanks! Looks good, can you squash the commits into one and update the changelog?

[ringods]

Never "squashed" commits. Any pointer how I can do that?

[carlossg]

http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html

from your branch you can do a git rebase -i master and set all but the first commit to squash instead of pick. Set a nice commit message and then you'll need to git push --force

[ringods]

Commits squashed. I came from a green build and now I get failures... Any idea what could cause the timeouts?

[carlossg]

Thanks, I need to rethink the timeouts in Travis, it's not very stable