Skip to content

Harden permissions for pipe used in SharedServer.#127239

Merged
cincuranet merged 1 commit intodotnet:mainfrom
cincuranet:tighter-perms
Apr 23, 2026
Merged

Harden permissions for pipe used in SharedServer.#127239
cincuranet merged 1 commit intodotnet:mainfrom
cincuranet:tighter-perms

Conversation

@cincuranet
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens Unix named-pipe (Unix domain socket) filesystem permissions when PipeOptions.CurrentUserOnly is used, by explicitly setting the socket file’s mode to user-only (0600). This aligns the on-disk endpoint permissions with the “current user only” intent and adds a regression test.

Changes:

  • Set the Unix domain socket file mode to UserRead | UserWrite when creating a SharedServer for CurrentUserOnly.
  • Add a Unix test validating the socket file mode when CurrentUserOnly is specified.
  • Minor refactors to use PipeOptions.HasFlag(...) in a few places.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs Applies 0600 permissions to the bound socket path for CurrentUserOnly; adds cleanup on failure; refactors option checks.
src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CurrentUserOnly.Unix.cs Adds a regression test asserting the created socket path has user-only permissions.

Comment thread src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs Outdated
Comment thread src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs Outdated
Comment thread src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Unix.cs Outdated
Copilot AI review requested due to automatic review settings April 22, 2026 09:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Copy link
Copy Markdown
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Thank you!

@cincuranet
Copy link
Copy Markdown
Contributor Author

/ba-g Failures are unrelated.

@cincuranet cincuranet merged commit dcc9f12 into dotnet:main Apr 23, 2026
88 of 100 checks passed
@cincuranet cincuranet added breaking-change Issue or PR that represents a breaking API or functional change over a previous release. needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet labels Apr 23, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

dotnet-policy-service Bot commented Apr 23, 2026

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

  1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
  2. Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

@cincuranet cincuranet removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.IO breaking-change Issue or PR that represents a breaking API or functional change over a previous release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants