Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable suid and atime on the /nix mount point on Darwin #12016

Merged
merged 1 commit into from
Dec 6, 2024

Conversation

grahamc
Copy link
Member

@grahamc grahamc commented 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.

Motivation

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

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
Copy link
Member Author

grahamc commented Dec 5, 2024

cc @abathur

@@ -463,7 +463,7 @@ EOF

EDITOR="$SCRATCH/ex_cleanroom_wrapper" _sudo "to add nix to fstab" "$@" <<EOF
:a
UUID=$uuid $escaped_mountpoint apfs rw,noauto,nobrowse,suid,owners
UUID=$uuid $escaped_mountpoint apfs rw,noauto,nobrowse,nosuid,noatime,owners
Copy link
Member

Choose a reason for hiding this comment

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

cc @risicle You might need to update documentation for macOS users in this case if they want to use your: https://github.com/risicle/nix-heuristic-gc/

@Mic92 Mic92 merged commit ab5a9cf into NixOS:master Dec 6, 2024
13 checks passed
Copy link
Contributor

mergify bot commented Dec 6, 2024

queue

🛑 The pull request has been merged manually

The pull request has been merged manually at ab5a9cf

Copy link
Contributor

mergify bot commented Dec 6, 2024

This pull request has been removed from the queue for the following reason: pull request dequeued.

Pull request #12016 has been dequeued. The pull request has been merged manually. The pull request has been merged manually at ab5a9cf.

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

@Mic92
Copy link
Member

Mic92 commented Dec 6, 2024

@grahamc thanks!

@grahamc grahamc deleted the patch-2 branch December 6, 2024 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants