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

Chmod build root to make it read-only #11448

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

noamraph
Copy link
Contributor

@noamraph noamraph commented Sep 9, 2024

Motivation

Currently, building Nix itself fails in single-user installation - see #11295. This is because some build steps run something like mkdir -p $HOME/.cache, and since $HOME is /homeless-shelter and / is writable, the directory /homeless-shelter is created, which may cause the builder to fail.

This PR just runs a chmod on the build root directory prior to chroot, to make it read-only. This seems to solve the issue.

Context

Priorities and Process

Add 👍 to pull requests you find important.

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

@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Sep 9, 2024
@@ -1970,6 +1970,9 @@ void LocalDerivationGoal::runChild()
if (rmdir("real-root") == -1)
throw SysError("cannot remove real-root directory");

// Make build root read-only, so `mkdir /homeless-shelter` would fail.
chmod_("/", 0555);
Copy link
Member

Choose a reason for hiding this comment

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

Could this break uid-range builds (e.g. tests/nixos/containers/systemd-nspawn.nix)? Those do expect the root to be writable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I guess it could.

I had another idea. If we actually mkdir /homeless-shelter and then make it read-only, and adjust the "home directory exists" error to be only triggered if the directory exists and is non-empty, I think it may solve the issue. Would you object to always having an empty /homeless-shelter directory in the sandbox?

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps this should then be controlled by a derivation-provided setting like an "advanced attribute"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants