install: label only the directories it creates by abendrothj · Pull Request #15333 · uutils/coreutils

abendrothj · 2026-10-14T05:14:12Z

With -Z or --context, install labelled directories after creating them, by walking up from the target and setting the context on every existing parent too, up to / for an absolute path. GNU labels only what it creates.

Directories are now created under the creation context, through uucore's FsCreateContext as mkdir does, so only new ones are labelled and nothing is relabelled afterwards. With -Z, each new directory gets the policy's default for its own name, as in GNU; for that, create_dir_all_safe takes a hook that runs before each missing directory is created. It also resolves a relative name whose parents are created by the same command. This also gives new directories under -Z the policy's default type, the directory half of #15091 (except GNU's tmp_t for an absolute name directly in /tmp). As in GNU 9.12, -d -Z still resets an existing target to its default context, and -d --context leaves an existing target alone. An unusable --context now fails before anything is done, as in GNU, also when the target directory exists. With SELinux off, no context is set, as in mkdir.

Checked as root on Fedora 44 (enforcing) against GNU 9.12, under /var/www and /home, for -d and -D with -Z and --context on new and existing directories, relative and absolute: labels and exit status match in all 22 cases. Added tests for -d and -D with --context, for the -Z reset of an existing directory, for relative names with missing parents, and for new parents of a .snapshots directory, which the policy types differently; each fails without the change.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

oech3 · 2026-10-14T05:15:57Z

+            match directory_creation_context(&target, b) {
+                Ok(guard) => guard,
+                Err(e) => {
+                    show!(e);


Is this stdout? or show_error!?

It's stderr: show! prints like show_error! and also sets the exit code to 1, which GNU returns here.

github-actions · 2026-10-14T05:43:19Z

GNU testsuite comparison:

GNU test failed: tests/install/install-Z-selinux. tests/install/install-Z-selinux is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/tail/truncate. tests/tail/truncate is passing on 'main'. Maybe you have to rebase?

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

codspeed · 2026-10-14T06:04:08Z

Merging this PR will not alter performance

1 warning, 1 note

⚠️ 118 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

ℹ️ 1 benchmark has no flamegraph

The flamegraph wasn't generated: most of the time was spent in runtime internals, which would leave nothing useful to analyze.

See affected benchmarks

✅ 339 untouched benchmarks
⏩ 132 skipped benchmarks¹

_{Comparing abendrothj:install-context-created-dirs (bb549c8) with main (ff0c589)²}

132 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
No successful run was found on main (732b9c5) during the generation of this report, so ff0c589 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Devel08 · 2026-10-14T11:22:28Z

+#[cfg(all(feature = "selinux", any(target_os = "linux", target_os = "android")))]
+fn test_selinux_invalid_context_existing_directory() {
+    if !uucore::selinux::is_selinux_enabled() {
+        println!("Skipping SELinux test: SELinux is not enabled");


I don't think there's any sense of using println! in tests, what do you think @xtqqczze?

I don't know, but we do the same in test_selinux.

sylvestre · 2026-10-14T19:39:23Z

+        let _selinux_guard = {
+            let target = dir_strip_dot_for_creation(path);
+            match directory_creation_context(&target, b) {
+                Ok(guard) => guard,


dir_strip_dot_for_creation(path) is computed again a few lines below, could be shared?

sylvestre · 2026-10-14T19:39:29Z

+            // Like GNU, -Z resets an existing target to its default context; --context doesn't.
+            #[cfg(all(feature = "selinux", any(target_os = "linux", target_os = "android")))]
+            if b.privileged && b.default_context && is_selinux_enabled() {
+                show_if_err!(set_selinux_security_context(path, None));


why not set_selinux_default_context(path) as before?
and please add a test for this -Z reset of an existing dir

set_selinux_default_context doesn't ask the policy: it keeps the dir's own context, so nothing gets reset. set_selinux_security_context(path, None) looks the default up. Added test_selinux_default_context_resets_existing_directory, which fails with the old call.

sylvestre · 2026-10-14T19:39:36Z

+    let mode = uucore::libc::S_IFDIR | DEFAULT_MODE as uucore::libc::mode_t;
+    uucore::selinux::FsCreateContext::new(path, Some(mode), get_context_for_selinux(b))
+        .map(Some)
+        .map_err(|e| uucore::error::USimpleError::new(1, e.to_string()))


please use InstallError::SelinuxContextFailed like the other selinux errors here

sylvestre · 2026-10-14T19:39:40Z

+// parent keeps its context.
+#[test]
+#[cfg(all(feature = "selinux", any(target_os = "linux", target_os = "android")))]
+fn test_selinux_context_only_on_created_directories() {


could you please split it into two tests, one for -d and one for -D?

sylvestre · 2026-10-14T19:39:44Z

+// -Z looks the name up in the policy, which lists absolute names only: a relative
+// name must get the same label as the absolute one.
+#[test]
+#[cfg(all(feature = "selinux", any(target_os = "linux", target_os = "android")))]


this belongs to #15336, please rebase once it is merged

Yes, I'll rebase once #15336 is in.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

sylvestre · 2026-10-22T22:06:34Z

-/// Traverses from the given starting path up to existing parent directories.
-/// Returns a vector of paths in reverse order (from parent to child).
+/// With -Z or --context, label the directories created while the returned guard lives.
+/// One policy lookup for `path` serves all of them; GNU looks up each component.


so with -Z the created parents get the leaf's type, e.g. install -d -Z /var/www/cgi-bin/x labels /var/www as cgi content?
GNU looks up each one, could we do the same?

Yes, good catch. Each new directory is now looked up by its own name, as GNU does (create_dir_all_safe takes a hook for it). Checked against GNU 9.12 with /var/www/…/cgi-bin/… as root.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

github-actions · 2026-10-23T09:42:46Z

Binary size comparison:

Individual binary size comparison VS main (threshold: >=5% AND >=4 KB).

Total size of compared binaries: 155.04 MB (-304 KB, -0.19%)

Significant per-binary changes:
  date     2.61 MB ->    2.30 MB  (-316 KB, -11.81%)

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

sylvestre · 2026-10-25T09:43:34Z

rebased on main; after #12715 create_dir_all_with now creates with DEFAULT_MODE, otherwise the umask 0 gave 0777 ancestors

Copilot AI balanced review requested due to automatic review settings October 14, 2026 05:14

Copilot AI reviewed Oct 14, 2026

oech3 reviewed Oct 14, 2026

View reviewed changes

abendrothj mentioned this pull request Oct 14, 2026

uucore: look up the -Z context of a relative name by its absolute name #15336

Merged

Copilot AI balanced review requested due to automatic review settings October 14, 2026 06:02

abendrothj force-pushed the install-context-created-dirs branch from 3ddad2f to 69c9757 Compare October 14, 2026 06:02

Copilot AI reviewed Oct 14, 2026

Devel08 reviewed Oct 14, 2026

View reviewed changes

sylvestre reviewed Oct 14, 2026

View reviewed changes

abendrothj force-pushed the install-context-created-dirs branch from 69c9757 to c28736c Compare October 14, 2026 22:08

Copilot AI balanced review requested due to automatic review settings October 14, 2026 22:08

Copilot AI reviewed Oct 14, 2026

This was referenced Oct 14, 2026

install: give files the policy's default context under -Z #15359

Draft

install: -Z gives new files and directories a different SELinux type than GNU #15091

Open

sylvestre reviewed Oct 22, 2026

View reviewed changes

Comment thread tests/by-util/test_install.rs Outdated

Copilot AI balanced review requested due to automatic review settings October 23, 2026 07:56

abendrothj force-pushed the install-context-created-dirs branch from c28736c to f145794 Compare October 23, 2026 07:56

Copilot AI reviewed Oct 23, 2026

install: label only the directories it creates

bb549c8

sylvestre force-pushed the install-context-created-dirs branch from f145794 to bb549c8 Compare October 25, 2026 09:31

Copilot AI balanced review requested due to automatic review settings October 25, 2026 09:31

Copilot AI reviewed Oct 25, 2026

Conversation

abendrothj commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

codspeed Bot commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

sylvestre commented Oct 25, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

abendrothj commented Oct 14, 2026 •

edited

Loading

codspeed Bot commented Oct 14, 2026 •

edited

Loading