Graham/fh 496 nix installer incorrectly validates fstab entries #1338
Conversation
This file contains 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
cole-h
reviewed
Dec 5, 2024
cole-h
reviewed
Dec 5, 2024
cole-h
reviewed
Dec 5, 2024
/nix should not contain suid/setgid binaries in general. We don't need atime either. At the time, I think we were wondering if nix-collect-garbage used atime. It doesn't. Having atime is a pretty big performance penalty, and macOS mounts most filesytems noatime already.
cole-h
reviewed
Dec 5, 2024
* Move the "prelude" into a suffix, which makes writing and updating the fstab a much simpler operation. The fstab on macOS, where this is used, appears to handle a suffix comment just fine. * Stop trying to detect foreign / native fstab entries. * Generally, cut out a lot of opportunities for failure by implementing this as a straightforward filter and map.
grahamc
force-pushed
the
graham/fh-496-nix-installer-incorrectly-validates-fstab-entries
branch
from
1dd8bf5 to
15ea3e6
Compare
cole-h
reviewed
Dec 5, 2024
cole-h
reviewed
Dec 5, 2024
grahamc
force-pushed
the
graham/fh-496-nix-installer-incorrectly-validates-fstab-entries
branch
from
3322941 to
3cb1128
Compare
cole-h
approved these changes
Dec 5, 2024
grahamc
added a commit
to grahamc/nix
that referenced
this pull request
Dec 5, 2024
The Determinate Nix Installer has set nosuid and noatime in DeterminateSystems/nix-installer#1338, and figured this perf and security improvement is worthy of upstreaming. The /nix volume shouldn't have setuid binaries anyway, and filesystems seem to generally be noatime on macOS. Further, the garbage collector doesn't use atime.
grahamc
deleted the
graham/fh-496-nix-installer-incorrectly-validates-fstab-entries
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Drastically simplify how we edit the fstab. Also, enable nosuid and noatime, since neither of those are necessary for /nix.
Checklist
cargo fmtnix buildnix flake checkValidating with
install.determinate.systemsIf a maintainer has added the
upload to s3label to this PR, it will become available for installation viainstall.determinate.systems: