CHEF-19255 Fix pipe broken issue

lib/train/transports/local.rb

@@ -229,11 +229,28 @@ def acquire_pipe
           at_exit { close rescue Errno::EIO }
 
           pipe = nil
+          ownership_verified = false
 
           # PowerShell needs time to create pipe.
           100.times do
+            unless ownership_verified
+              owner, current_user, is_owner = pipe_owned_by_current_user?(pipe_name)
+              if owner.nil?
+                sleep 0.1
+                next
+              end
+
+              unless is_owner
+                raise PipeError, "Unauthorized user '#{current_user}' tried to connect to pipe '#{pipe_name}'. Pipe is owned by '#{owner}'."
+              end
+
+              ownership_verified = true
+            end
+
             pipe = open("//./pipe/#{pipe_name}", "r+")
             break
+          rescue PipeError
+            raise
           rescue
             sleep 0.1
           end
@@ -246,17 +263,26 @@ def start_pipe_server(pipe_name)
 
           script = <<-EOF
             $ErrorActionPreference = 'Stop'
+            $ProgressPreference = 'SilentlyContinue'
+            $user = [System.Security.Principal.WindowsIdentity]::GetCurrent().Name
+            $pipeSecurity = New-Object System.IO.Pipes.PipeSecurity
+            $rule = New-Object System.IO.Pipes.PipeAccessRule($user, "FullControl", "Allow")
+            $pipeSecurity.AddAccessRule($rule)
+            $pipeServer = New-Object System.IO.Pipes.NamedPipeServerStream('#{pipe_name}', [System.IO.Pipes.PipeDirection]::InOut, 1, [System.IO.Pipes.PipeTransmissionMode]::Byte, [System.IO.Pipes.PipeOptions]::None, 4096, 4096, $pipeSecurity)
+
+            $pipeServer.WaitForConnection()
 
-            $pipeServer = New-Object System.IO.Pipes.NamedPipeServerStream('#{pipe_name}')
             $pipeReader = New-Object System.IO.StreamReader($pipeServer)
             $pipeWriter = New-Object System.IO.StreamWriter($pipeServer)
 
-            $pipeServer.WaitForConnection()
-
             # Create loop to receive and process user commands/scripts
             $clientConnected = $true
             while($clientConnected) {
               $input = $pipeReader.ReadLine()
+              if ($input -eq $null) {
+                $clientConnected = $false
+                break
+              }
               $command = [System.Text.Encoding]::UTF8.GetString([System.Convert]::FromBase64String($input))
 
               # Execute user command/script and convert result to JSON
@@ -278,8 +304,14 @@ def start_pipe_server(pipe_name)
 
               # Encode JSON in Base64 and write to pipe
               $encodedResult = [System.Convert]::ToBase64String([System.Text.Encoding]::UTF8.GetBytes($resultJSON))
-              $pipeWriter.WriteLine($encodedResult)
-              $pipeWriter.Flush()
+              try {
+                $pipeWriter.WriteLine($encodedResult)
+                $pipeWriter.Flush()
+              } catch [System.IO.IOException] {
+                # Pipe was closed by client, exit gracefully
+                $clientConnected = $false
+                break
+              }
             }
           EOF
 
@@ -288,6 +320,29 @@ def start_pipe_server(pipe_name)
           cmd = "#{@powershell_cmd} -NoProfile -ExecutionPolicy bypass -NonInteractive -EncodedCommand #{base64_script}"
           Process.create(command_line: cmd).process_id
         end
+
+        def current_windows_user
+          user = `powershell -Command "[System.Security.Principal.WindowsIdentity]::GetCurrent().Name"`.strip
+          if user.nil? || user.empty?
+            user = `whoami`.strip
+          end
+          if user.nil? || user.empty?
+            raise "Unable to determine current Windows user"
+          end
+
+          user
+        end
+
+        # Verify pipe ownership before connecting
+        def pipe_owned_by_current_user?(pipe_name)
+          exists = `powershell -Command "Test-Path \\\\.\\pipe\\#{pipe_name}"`.strip.downcase == "true"
+          current_user = current_windows_user
+          return [nil, current_user, false] unless exists
+
+          owner = `powershell -Command "(Get-Acl \\\\.\\pipe\\#{pipe_name}).Owner" 2>&1`.strip
+          is_owner = !owner.nil? && !current_user.nil? && owner.casecmp(current_user) == 0
+          [owner, current_user, is_owner]
+        end
       end
     end
   end

test/unit/transports/local_test.rb

@@ -182,4 +182,122 @@ def mock_run_cmd(cmd, &block)
       connection.run_command("not actually executed")
     end
   end
+
+  describe "WindowsPipeRunner" do
+    let(:pipe_runner_class) { Train::Transports::Local::Connection::WindowsPipeRunner }
+
+    describe "#current_windows_user" do
+      let(:runner) do
+        pipe_runner_class.allocate.tap do |r|
+          r.instance_variable_set(:@powershell_cmd, "powershell")
+        end
+      end
+
+      it "returns user from WindowsIdentity when available" do
+        runner.expects(:`).with('powershell -Command "[System.Security.Principal.WindowsIdentity]::GetCurrent().Name"').returns("DOMAIN\\testuser\n")
+        result = runner.send(:current_windows_user)
+        _(result).must_equal "DOMAIN\\testuser"
+      end
+
+      it "falls back to whoami when WindowsIdentity returns empty" do
+        runner.expects(:`).with('powershell -Command "[System.Security.Principal.WindowsIdentity]::GetCurrent().Name"').returns("\n")
+        runner.expects(:`).with("whoami").returns("domain\\fallbackuser\n")
+        result = runner.send(:current_windows_user)
+        _(result).must_equal "domain\\fallbackuser"
+      end
+
+      it "falls back to whoami when WindowsIdentity returns nil-like value" do
+        runner.expects(:`).with('powershell -Command "[System.Security.Principal.WindowsIdentity]::GetCurrent().Name"').returns("")
+        runner.expects(:`).with("whoami").returns("localuser\n")
+        result = runner.send(:current_windows_user)
+        _(result).must_equal "localuser"
+      end
+
+      it "raises error when both methods fail to return a user" do
+        runner.expects(:`).with('powershell -Command "[System.Security.Principal.WindowsIdentity]::GetCurrent().Name"').returns("")
+        runner.expects(:`).with("whoami").returns("")
+        _ { runner.send(:current_windows_user) }.must_raise RuntimeError
+      end
+    end
+
+    describe "#pipe_owned_by_current_user?" do
+      let(:runner) do
+        pipe_runner_class.allocate.tap do |r|
+          r.instance_variable_set(:@powershell_cmd, "powershell")
+        end
+      end
+
+      it "returns [nil, current_user, false] when pipe does not exist" do
+        runner.expects(:`).with('powershell -Command "Test-Path \\\\.\\pipe\\test_pipe"').returns("false\n")
+        runner.expects(:current_windows_user).returns("DOMAIN\\testuser")
+        owner, current_user, is_owner = runner.send(:pipe_owned_by_current_user?, "test_pipe")
+        _(owner).must_be_nil
+        _(current_user).must_equal "DOMAIN\\testuser"
+        _(is_owner).must_equal false
+      end
+
+      it "returns [owner, current_user, true] when pipe exists and is owned by current user" do
+        runner.expects(:`).with('powershell -Command "Test-Path \\\\.\\pipe\\test_pipe"').returns("true\n")
+        runner.expects(:current_windows_user).returns("DOMAIN\\testuser")
+        runner.expects(:`).with('powershell -Command "(Get-Acl \\\\.\\pipe\\test_pipe).Owner" 2>&1').returns("DOMAIN\\testuser\n")
+        owner, current_user, is_owner = runner.send(:pipe_owned_by_current_user?, "test_pipe")
+        _(owner).must_equal "DOMAIN\\testuser"
+        _(current_user).must_equal "DOMAIN\\testuser"
+        _(is_owner).must_equal true
+      end
+
+      it "returns [owner, current_user, false] when pipe is owned by different user" do
+        runner.expects(:`).with('powershell -Command "Test-Path \\\\.\\pipe\\test_pipe"').returns("true\n")
+        runner.expects(:current_windows_user).returns("DOMAIN\\testuser")
+        runner.expects(:`).with('powershell -Command "(Get-Acl \\\\.\\pipe\\test_pipe).Owner" 2>&1').returns("DOMAIN\\otheruser\n")
+        owner, current_user, is_owner = runner.send(:pipe_owned_by_current_user?, "test_pipe")
+        _(owner).must_equal "DOMAIN\\otheruser"
+        _(current_user).must_equal "DOMAIN\\testuser"
+        _(is_owner).must_equal false
+      end
+
+      it "performs case-insensitive comparison for ownership" do
+        runner.expects(:`).with('powershell -Command "Test-Path \\\\.\\pipe\\test_pipe"').returns("true\n")
+        runner.expects(:current_windows_user).returns("domain\\TESTUSER")
+        runner.expects(:`).with('powershell -Command "(Get-Acl \\\\.\\pipe\\test_pipe).Owner" 2>&1').returns("DOMAIN\\testuser\n")
+        _owner, _current_user, is_owner = runner.send(:pipe_owned_by_current_user?, "test_pipe")
+        _(is_owner).must_equal true
+      end
+    end
+
+    describe "#acquire_pipe" do
+      let(:runner) do
+        pipe_runner_class.allocate.tap do |r|
+          r.instance_variable_set(:@powershell_cmd, "powershell")
+          r.instance_variable_set(:@server_pid, nil)
+        end
+      end
+
+      before do
+        runner.stubs(:require).with("win32/process").returns(true)
+      end
+
+      it "raises PipeError when pipe is owned by unauthorized user" do
+        runner.expects(:start_pipe_server).with(anything).returns(12345)
+        runner.expects(:pipe_owned_by_current_user?).with(anything).returns(["DOMAIN\\otheruser", "DOMAIN\\testuser", false])
+        runner.stubs(:close)
+
+        _ { runner.send(:acquire_pipe) }.must_raise Train::Transports::Local::PipeError
+      end
+
+      it "waits for pipe to be created before verifying ownership" do
+        mock_pipe = mock("pipe")
+        runner.expects(:start_pipe_server).with(anything).returns(12345)
+        runner.expects(:pipe_owned_by_current_user?).with(anything).twice.returns(
+          [nil, "DOMAIN\\testuser", false],
+          ["DOMAIN\\testuser", "DOMAIN\\testuser", true]
+        )
+        runner.expects(:open).with(anything, "r+").returns(mock_pipe)
+        runner.stubs(:close)
+
+        result = runner.send(:acquire_pipe)
+        _(result).must_equal mock_pipe
+      end
+    end
+  end
 end

test/windows/local_test.rb

@@ -272,6 +272,38 @@ def override_platform(platform)
     end
   end
 
+  describe "pipe security and ownership verification" do
+    let(:runner) { conn.instance_variable_get(:@runner) }
+
+    it "current_windows_user returns a valid username" do
+      skip "ShellRunner in use" unless runner.is_a?(Train::Transports::Local::Connection::WindowsPipeRunner)
+
+      user = runner.send(:current_windows_user)
+      _(user).wont_be_nil
+      _(user).wont_be_empty
+      _(user).must_match(/\S+/)
+    end
+
+    it "handles multiple sequential commands without pipe errors" do
+      skip "ShellRunner in use" unless runner.is_a?(Train::Transports::Local::Connection::WindowsPipeRunner)
+
+      5.times do |i|
+        cmd = conn.run_command("Write-Output 'iteration #{i}'")
+        _(cmd.stdout).must_equal "iteration #{i}\r\n"
+        _(cmd.exit_status).must_equal 0
+      end
+    end
+
+    it "handles large output without pipe errors" do
+      skip "ShellRunner in use" unless runner.is_a?(Train::Transports::Local::Connection::WindowsPipeRunner)
+
+      cmd = conn.run_command("1..100 | ForEach-Object { Write-Output \"Line $_\" }")
+      _(cmd.exit_status).must_equal 0
+      _(cmd.stdout).must_include "Line 1"
+      _(cmd.stdout).must_include "Line 100"
+    end
+  end
+
   after do
     # close the connection
     conn.close