Support readLinkAt and openFileEnsureBeneathNoSymlinks on Windows too#15060
Merged
Ericson2314 merged 1 commit intomasterfrom Feb 4, 2026
Merged
Support readLinkAt and openFileEnsureBeneathNoSymlinks on Windows too#15060Ericson2314 merged 1 commit intomasterfrom
readLinkAt and openFileEnsureBeneathNoSymlinks on Windows too#15060Ericson2314 merged 1 commit intomasterfrom
Conversation
Ericson2314
force-pushed
the
read-link-at
branch
from
3f7b09d to
d8ff169
Compare
… too This means that `RestoreSink` can work in the TOCTOU-resilliant way on Windows too. And it also bodes will for the upcoming OS source accessor improvements. A few misc little refactors around error handling and whatnot are done along the way too. (No more attempt to support pre Windows Vista! lol.) This cannot be realiably automatically tested until we have a newer version of Wine, but it does build, so I am inclined to say we just try it for now.
Ericson2314
force-pushed
the
read-link-at
branch
from
d8ff169 to
1100c9d
Compare
Contributor
|
FWIW I can poke at wine 10.20 |
Ericson2314
added a commit
to obsidiansystems/nix
that referenced
this pull request
Jan 30, 2026
…em meta data This should not happen now, but instead happen after - NixOS#15119 - NixOS#15060 - Sergei's upcoming new `Descriptor`-based `SourceAccessor` I suspect what we'll want to do is expose that source accessor after all, so we can have some extra methods to get at the underlying file descriptors. (Or, conversely, maybe this won't be necessary, because enough of the underlying logic will be factored into `file-descriptor.hh` functions that the `SourceAccessor` itself will be a small wrapper.) Either way, at that point we'll not be duplicating stuff here, nor will be lacking a foundation on Windows, and we can then finish the job.
edolstra
approved these changes
Feb 4, 2026
xokdvium
reviewed
Feb 4, 2026
Comment on lines
+172
to
+180
| /** | ||
| * Get the path associated with a file handle. | ||
| * | ||
| * @note One MUST only use this for error handling, because it creates | ||
| * TOCTOU issues. We don't mind if error messages point to out of date | ||
| * paths (that is a rather trivial TOCTOU --- the error message is best | ||
| * effort) but for anything else we do. | ||
| */ | ||
| std::filesystem::path handleToPath(Descriptor handle); |
Contributor
There was a problem hiding this comment.
Btw we could implement it for Linux/macos too and improve error messages when working with FDs
Member
Author
There was a problem hiding this comment.
Yes, I agree we should do that.
Ericson2314
added a commit
to obsidiansystems/nix
that referenced
this pull request
Feb 5, 2026
…em meta data This should not happen now, but instead happen after - NixOS#15119 - NixOS#15060 - Sergei's upcoming new `Descriptor`-based `SourceAccessor` I suspect what we'll want to do is expose that source accessor after all, so we can have some extra methods to get at the underlying file descriptors. (Or, conversely, maybe this won't be necessary, because enough of the underlying logic will be factored into `file-descriptor.hh` functions that the `SourceAccessor` itself will be a small wrapper.) Either way, at that point we'll not be duplicating stuff here, nor will be lacking a foundation on Windows, and we can then finish the job.
Ericson2314
added a commit
to obsidiansystems/nix
that referenced
this pull request
Feb 5, 2026
…em meta data This should not happen now, but instead happen after - NixOS#15119 - NixOS#15060 - Sergei's upcoming new `Descriptor`-based `SourceAccessor` I suspect what we'll want to do is expose that source accessor after all, so we can have some extra methods to get at the underlying file descriptors. (Or, conversely, maybe this won't be necessary, because enough of the underlying logic will be factored into `file-descriptor.hh` functions that the `SourceAccessor` itself will be a small wrapper.) Either way, at that point we'll not be duplicating stuff here, nor will be lacking a foundation on Windows, and we can then finish the job.
Ericson2314
added a commit
to obsidiansystems/nix
that referenced
this pull request
Feb 16, 2026
…em meta data This should not happen now, but instead happen after - NixOS#15119 - NixOS#15060 - Sergei's upcoming new `Descriptor`-based `SourceAccessor` I suspect what we'll want to do is expose that source accessor after all, so we can have some extra methods to get at the underlying file descriptors. (Or, conversely, maybe this won't be necessary, because enough of the underlying logic will be factored into `file-descriptor.hh` functions that the `SourceAccessor` itself will be a small wrapper.) Either way, at that point we'll not be duplicating stuff here, nor will be lacking a foundation on Windows, and we can then finish the job.
Ericson2314
added a commit
to obsidiansystems/nix
that referenced
this pull request
Feb 16, 2026
…em meta data This should not happen now, but instead happen after - NixOS#15119 - NixOS#15060 - Sergei's upcoming new `Descriptor`-based `SourceAccessor` I suspect what we'll want to do is expose that source accessor after all, so we can have some extra methods to get at the underlying file descriptors. (Or, conversely, maybe this won't be necessary, because enough of the underlying logic will be factored into `file-descriptor.hh` functions that the `SourceAccessor` itself will be a small wrapper.) Either way, at that point we'll not be duplicating stuff here, nor will be lacking a foundation on Windows, and we can then finish the job.
Ericson2314
added a commit
to obsidiansystems/nix
that referenced
this pull request
Feb 16, 2026
…em meta data This should not happen now, but instead happen after - NixOS#15119 - NixOS#15060 - Sergei's upcoming new `Descriptor`-based `SourceAccessor` I suspect what we'll want to do is expose that source accessor after all, so we can have some extra methods to get at the underlying file descriptors. (Or, conversely, maybe this won't be necessary, because enough of the underlying logic will be factored into `file-descriptor.hh` functions that the `SourceAccessor` itself will be a small wrapper.) Either way, at that point we'll not be duplicating stuff here, nor will be lacking a foundation on Windows, and we can then finish the job.
Ericson2314
added a commit
to obsidiansystems/nix
that referenced
this pull request
Feb 18, 2026
…em meta data This should not happen now, but instead happen after - NixOS#15119 - NixOS#15060 - Sergei's upcoming new `Descriptor`-based `SourceAccessor` I suspect what we'll want to do is expose that source accessor after all, so we can have some extra methods to get at the underlying file descriptors. (Or, conversely, maybe this won't be necessary, because enough of the underlying logic will be factored into `file-descriptor.hh` functions that the `SourceAccessor` itself will be a small wrapper.) Either way, at that point we'll not be duplicating stuff here, nor will be lacking a foundation on Windows, and we can then finish the job.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
This means that
RestoreSinkcan work in the TOCTOU-resilliant way on Windows too. And it also bodes will for the upcoming OS source accessor improvements.A few misc little refactors around error handling and whatnot are done along the way too. (No more attempt to support pre Windows Vista! lol.)
Context
This cannot be reliably automatically tested until we have a newer version of Wine, but it does build, so I am inclined to say we just try it for now.
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.