Merge "warden: Sanitize config to CREATE before using it"

warden/lib/warden/container/linux.rb

@@ -44,13 +44,13 @@ def env_command
         "env #{env.map { |k, v| "#{k}=#{v}" }.join(" ")}"
       end
 
-      def do_create(config={})
-        check_create_config(config)
+      def do_create(config = {})
+        config = sanitize_config(config)
 
         sh "#{env_command} #{root_path}/create.sh #{handle}", :timeout => nil
         debug "container created"
 
-        create_bind_mount_script(config["bind_mounts"] || {})
+        create_bind_mount_script(config)
         debug "wrote bind mount script"
 
         sh "#{container_path}/start.sh", :timeout => nil
@@ -112,16 +112,53 @@ def do_copy_out(src_path, dst_path, owner=nil)
 
       private
 
-      def check_create_config(config)
-        if config["bind_mounts"]
-          config["bind_mounts"].each do |src_path, dest_info|
-            unless dest_info["mode"].nil? || ["rw", "ro"].include?(dest_info["mode"])
-              emsg = "Invalid mode for bind mount '#{src_path}'." \
-                     + " Must be one of 'rw, ro'."
-              raise WardenError.new(emsg)
+      def sanitize_config(config)
+        bind_mounts = config.delete("bind_mounts")
+
+        # Raise when it is not an Array
+        if !bind_mounts.is_a?(Array)
+          raise WardenError.new("Expected `bind_mounts` to hold an array.")
+        end
+
+        # Transform entries
+        bind_mounts = bind_mounts.map do |src_path, dst_path, options|
+          options ||= {}
+
+          if !src_path.is_a?(String)
+            raise WardenError.new("Expected `src_path` to be a string.")
+          end
+
+          if !dst_path.is_a?(String)
+            raise WardenError.new("Expected `dst_path` to be a string.")
+          end
+
+          if !options.is_a?(Hash)
+            raise WardenError.new("Expected `options` to be a hash.")
+          end
+
+          # Fix up destination path to be an absolute path inside the union
+          dst_path = File.join(container_path,
+                                "union",
+                                dst_path.slice(1, dst_path.size - 1))
+
+          # Check that the mount mode -- if given -- is "ro" or "rw"
+          if options.has_key?("mode")
+            unless ["ro", "rw"].include?(options["mode"])
+              raise WardenError, [
+                  %{Invalid mode for bind mount "%s".} % src_path,
+                  %{Must be one of "ro", "rw".}
+                ].join(" ")
             end
           end
+
+          [src_path, dst_path, options]
         end
+
+        # Filter nil entries
+        bind_mounts = bind_mounts.compact
+
+        # Return sanitized config
+        { "bind_mounts" => bind_mounts }
       end
 
       def perform_rsync(src_path, dst_path)
@@ -135,23 +172,13 @@ def perform_rsync(src_path, dst_path)
         sh(cmd, :timeout => nil)
       end
 
-      def create_bind_mount_script(bind_mounts)
-        params = {"bind_mounts" => bind_mounts.dup}
-
-        # Fix up destination paths so that they are absolute paths inside the union
-        params["bind_mounts"].each_value do |mount_info|
-          path = mount_info["path"]
-          mount_info["path"] = File.join(container_path,
-                                         "union",
-                                         path.slice(1, path.size - 1))
-        end
-
+      def create_bind_mount_script(config)
+        params = config.dup
         script_contents = self.class.bind_mount_script_template.result(binding())
         script_path = File.join(container_path, "setup-bind-mounts.sh")
         File.open(script_path, 'w+') {|f| f.write(script_contents) }
         sh "chmod 0700 #{script_path}"
       end
-
     end
   end
 end

warden/lib/warden/repl.rb

@@ -84,11 +84,8 @@ def make_create_config(args)
       args.each do |arg|
         if arg =~ /^bind_mount:(.*)/
           src, dst, mode = $1.split(",")
-          config["bind_mounts"] ||= {}
-          config["bind_mounts"][src] = {
-            "path" => dst,
-            "mode" => mode,
-          }
+          config["bind_mounts"] ||= []
+          config["bind_mounts"].push [src, dst, { "mode" => mode }]
         else
           raise "Unknown argument: #{arg}"
         end

warden/root/linux/setup-bind-mounts.erb

@@ -3,10 +3,10 @@
 set -o nounset
 set -o errexit
 
-<% for source_path, dest_info in params["bind_mounts"] %>
-mkdir -p <%= dest_info["path"] %>
-mount --bind <%= source_path %> <%= dest_info["path"] %>
-<% if dest_info["mode"] %>
-mount -o remount,<%= dest_info["mode"] %> <%= dest_info["path"] %>
+<% for src_path, dst_path, options in params["bind_mounts"] %>
+mkdir -p <%= dst_path %>
+mount --bind <%= src_path %> <%= dst_path %>
+<% if options["mode"] %>
+mount -o remount,<%= options["mode"] %> <%= dst_path %>
+<% end %>
 <% end %>
-<% end %>

warden/spec/server_linux_spec.rb

@@ -137,9 +137,9 @@
       FileUtils.chmod_R(0777, @tmpdir)
       @bind_mount_path = "/tmp/bind_mounted"
       @config = {
-        "bind_mounts" => {
-          @tmpdir => {"path" => @bind_mount_path}
-        }
+        "bind_mounts" => [
+          [@tmpdir, @bind_mount_path, {}]
+        ]
       }
     end
 
@@ -148,7 +148,7 @@
     end
 
     it "should raise an error if an invalid mode is supplied" do
-      @config["bind_mounts"][@tmpdir]["mode"] = "invalid"
+      @config["bind_mounts"][0][2]["mode"] = "invalid"
       expect do
         handle = client.create(@config)
       end.to raise_error(/Invalid mode/)
@@ -173,7 +173,7 @@
     end
 
     it "should support bind mounting paths with different permissions" do
-      @config["bind_mounts"][@tmpdir]["mode"] = "ro"
+      @config["bind_mounts"][0][2]["mode"] = "ro"
       handle = client.create(@config)
 
       result = client.run(handle, "touch #{@bind_mount_path}/test")