Skip to content

install: fix -D data-loss when destination parent is the root directory#13259

Open
koopatroopa787 wants to merge 1 commit into
uutils:mainfrom
koopatroopa787:fix-install-D-root-parent
Open

install: fix -D data-loss when destination parent is the root directory#13259
koopatroopa787 wants to merge 1 commit into
uutils:mainfrom
koopatroopa787:fix-install-D-root-parent

Conversation

@koopatroopa787

Copy link
Copy Markdown
Contributor

Summary

install -D src /file silently deletes the source file and then
fails, leaving the user with neither the source nor the destination.

Root cause

When the target is a single-component absolute path like /file,
target.parent() returns "/". Before calling create_dir_all_safe,
the code strips trailing slashes from that parent:

while trimmed_bytes.ends_with(b"/") {
    trimmed_bytes = &trimmed_bytes[..trimmed_bytes.len() - 1];
}

For "/" this strips ALL bytes and produces "".
find_existing_ancestor("") maps an empty path to "." (current
directory), so create_dir_all_safe returns a DirFd pointing at CWD
instead of /.

The subsequent unlink_at(cwd_fd, "file") then removes ./file — the
source — rather than any existing destination. The copy step fails
because the source is gone, and the user ends up with nothing.

Fix

Skip the trailing-slash strip when the result would be empty (i.e. the
path was entirely slashes). The root directory "/" is left as-is, so
create_dir_all_safe opens / as the parent fd and later
unlink_at(root_fd, "file") targets the intended destination.

Changes

  • src/uu/install/src/install.rs: guard the slash-strip so an
    all-slash path ("/") is kept intact.
  • tests/by-util/test_install.rs: regression test verifying the source
    file survives when install -D src /dest fails (non-root execution).

Fixes #13232

`install -D src /file` strips trailing slashes from the destination's
parent directory before calling `create_dir_all_safe`.  For a parent of
`"/"` the loop removes ALL bytes and produces `""`, which
`find_existing_ancestor` (inside `create_dir_all_safe`) maps to `"."` —
the current directory.  The resulting `DirFd` points at CWD, so the
subsequent `unlink_at(cwd, "file")` deletes the source file, and the
copy fails leaving the user with no file at all.

Fix: when the post-stripping result would be empty (i.e. the original
path was entirely slashes), keep the path unchanged so the root
directory is opened as the parent fd instead of the current directory.

Adds a regression test that verifies the source file is preserved when
`install -D src /single-component-dest` fails (as a non-root user).

Fixes uutils#13232
Copilot AI review requested due to automatic review settings July 2, 2026 19:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

@codspeed-hq

codspeed-hq Bot commented Jul 2, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 4.84%

⚡ 1 improved benchmark
✅ 330 untouched benchmarks
⏩ 46 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation sort_ascii_utf8_locale 16.1 ms 15.4 ms +4.84%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing koopatroopa787:fix-install-D-root-parent (a999c25) with main (eae191c)

Open in CodSpeed

Footnotes

  1. 46 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.

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/date/date-locale-hour (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tail/retry (passes in this run but fails in the 'main' branch)

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.

install -D file /file fails and loses the file when the destination's parent is /

2 participants