Skip to content

Conversation

@sathish-progress
Copy link
Contributor

This pull request improves the security and reliability of named pipe communication in the lib/train/transports/local.rb transport by enforcing strict pipe ownership checks and handling pipe closure scenarios more gracefully. The main focus is on ensuring only the intended Windows user can access the named pipe and making the PowerShell server more robust.

Description

Security and Access Control:

  • Added a verification step in acquire_pipe to ensure that only the current Windows user can connect to the named pipe. If the pipe is owned by another user, an explicit error is raised to prevent unauthorized access.
  • Modified the PowerShell script in start_pipe_server to explicitly set pipe security, granting full control only to the current user when creating the named pipe.
  • Introduced helper methods current_windows_user and pipe_owned_by_current_user? to reliably determine the current user and verify pipe ownership before connecting.

Reliability Improvements:

  • Updated the PowerShell server loop to detect when the client disconnects (i.e., receives a null input), allowing the server to exit gracefully instead of hanging or throwing errors.
  • Wrapped pipe writing in a try/catch block to handle cases where the client closes the pipe unexpectedly, ensuring the server exits cleanly on System.IO.IOException.

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New content (non-breaking change)
  • Breaking change (a content change which would break existing functionality or processes)

Checklist:

  • I have read the CONTRIBUTING document.

Signed-off-by: Sathish Babu <sbabu@progress.com>
Signed-off-by: Sathish Babu <sbabu@progress.com>
Signed-off-by: Sathish Babu <sbabu@progress.com>
…ions

Signed-off-by: Sathish Babu <sbabu@progress.com>
Signed-off-by: Sathish Babu <sbabu@progress.com>
…d flushing

Signed-off-by: Sathish Babu <sbabu@progress.com>
Signed-off-by: Sathish Babu <sbabu@progress.com>
Copy link
Contributor

@Vasu1105 Vasu1105 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but can we add test coverage?

@Vasu1105 Vasu1105 changed the title Fix pipe broken issue CHEF-19255 Fix pipe broken issue Jan 9, 2026
…uisition

Signed-off-by: Sathish Babu <sbabu@progress.com>
…Runner

Signed-off-by: Sathish Babu <sbabu@progress.com>
@sathish-progress sathish-progress marked this pull request as ready for review January 10, 2026 06:50
@sathish-progress sathish-progress requested a review from a team as a code owner January 10, 2026 06:50
…write and reader

Signed-off-by: Sathish Babu <sbabu@progress.com>
Signed-off-by: Sathish Babu <sbabu@progress.com>
Copy link
Contributor

@clintoncwolfe clintoncwolfe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Closely read, looks good

@sathish-progress sathish-progress merged commit cc6ce2b into main Jan 14, 2026
30 of 31 checks passed
@sathish-progress sathish-progress deleted the fix-pipe-broken-issue branch January 14, 2026 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants