Skip to content

Comments

Prepare for FreeBSD sandboxing support#13281

Merged
Ericson2314 merged 1 commit intomasterfrom
freebsd-utils
May 27, 2025
Merged

Prepare for FreeBSD sandboxing support#13281
Ericson2314 merged 1 commit intomasterfrom
freebsd-utils

Conversation

@Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented May 27, 2025

Motivation

This is the utility changes from #9968, which were easier to rebase first.

Context

I (@Ericson2314) didn't write this code; I just rebased it.


Add 👍 to pull requests you find important.

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

@Ericson2314 Ericson2314 requested a review from edolstra as a code owner May 27, 2025 18:53
@github-actions github-actions bot added the new-cli Relating to the "nix" command label May 27, 2025
This is the utility changes from #9968, which were easier to rebase
first.

I (@Ericson2314) didn't write this code; I just rebased it.

Co-Authored-By: Artemis Tosini <me@artem.ist>
Co-Authored-By: Audrey Dutcher <audrey@rhelmot.io>
Comment on lines +372 to +378
#ifdef __FreeBSD__
#define MOUNTEDPATHS_PARAM , std::set<Path> &mountedPaths
#define MOUNTEDPATHS_ARG , mountedPaths
#else
#define MOUNTEDPATHS_PARAM
#define MOUNTEDPATHS_ARG
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity: is there a reason to do this ugly define stuff rather than making it std::option<std::set<Path>> or something similar?

Copy link
Member Author

Choose a reason for hiding this comment

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

That would create a bunch of runtime failure modes. non-freebsd would have to assert it is not present, and freebsd would to assert it is present

@Ericson2314 Ericson2314 merged commit de71ceb into master May 27, 2025
26 checks passed
@Ericson2314 Ericson2314 deleted the freebsd-utils branch May 27, 2025 19:37
@edolstra
Copy link
Member

I think this may have broken the FreeBSD build: https://hydra.nixos.org/build/298336685

@Ericson2314
Copy link
Member Author

Ericson2314 commented May 28, 2025

Oh, oops! It passed locally in shell but I see I forgot to commit that file forgot to include in the fileset whitelist.

static bool b = (settings.buildUsersGroup != "" || settings.autoAllocateUids) && isRootUser();
return b;
#elif defined(__APPLE__)
#elif defined(__APPLE__) && defined(__FreeBSD__)
Copy link
Member

Choose a reason for hiding this comment

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

Missed this in review, but fixed in #13455

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/critical-security-issue-in-nix-2-30-on-macos/66506/1

@roberth roberth added the backports rejected PR should not be backported (or at least not in full, or unless an overriding need arises) label Jul 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backports rejected PR should not be backported (or at least not in full, or unless an overriding need arises) new-cli Relating to the "nix" command

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants