Skip to content

mv: decide overwrite prompts by effective write access - #15282

Open
MuntasirSZN wants to merge 2 commits into
uutils:mainfrom
MuntasirSZN:mv-fix-nixos
Open

MuntasirSZN wants to merge 2 commits into
uutils:mainfrom
MuntasirSZN:mv-fix-nixos

Conversation

@MuntasirSZN

Copy link
Copy Markdown
Contributor

mv checked the owner write bit instead of write access, so root was prompted
before overwriting a 0555 file, and a symlink to an unwritable file drew a
spurious prompt.

is_writable now uses accessat with AT_EACCESS and treats symlink destinations as
writable, and the prompt wording comes from that same check. Tests cover both.

This failed in NixOS very badly.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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.

@anastygnome

Copy link
Copy Markdown
Contributor

@cakebaker I've implemented it in the faccessat.rs file in the test until. Maybe we should make it a common function in uucore

@anastygnome

Copy link
Copy Markdown
Contributor

refs #15294

@MuntasirSZN

Copy link
Copy Markdown
Contributor Author

@anastygnome should I close this pr?

@MuntasirSZN

Copy link
Copy Markdown
Contributor Author

Btw your pr is better in every way (though some conflicts)

@anastygnome

Copy link
Copy Markdown
Contributor

@MuntasirSZN you can close this but can you please make sure I didn't forget a case in the MV commit on my pr?

@MuntasirSZN

Copy link
Copy Markdown
Contributor Author

@anastygnome Symlinks: is_writable still resolves the link and accessat follows it, so a symlink to an unwritable file still draws a spurious prompt; that case reproduces as a plain user. Wording: get_interactive_prompt still keys off 0o200, so the prompt text can disagree with the prompt decision.

@anastygnome

anastygnome commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

@MuntasirSZN Please be cleare I didnt get that, do you have a clear reproducing scenario?

@MuntasirSZN

Copy link
Copy Markdown
Contributor Author

@anastygnome

❯ mv --version
mv (GNU coreutils) 9.9
Packaged by https://nixos.org
Copyright (C) 2025 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by Mike Parker, David MacKenzie, and Jim Meyering.
❯ cd /tmp && rm -rf repro1 && mkdir repro1 && cd repro1
direnv: unloading
❯ echo OLD > real && chmod 0555 real && ln -s real link && echo NEW > src
❯ printf 'n\n' | script -qec "/tmp/opencode/mv-pr15294 src link" /dev/null
n
mv-pr15294: replace 'link', overriding mode 0555 (r-xr-xr-x)? ⏎                                                                                                                           ❯ # but gnu
❯ cd /tmp && rm -rf repro1 && mkdir repro1 && cd repro1
❯ echo OLD > real && chmod 0555 real && ln -s real link && echo NEW > src
❯ script -qec "mv src link"
❯ # no prompt!
❯ cd /tmp && rm -rf repro2 && mkdir repro2 && cd repro2 && echo NEW > src
❯ ls -l /etc/passwd
-rw-r--r-- 1 root root 4658 Oct 13 20:21 /etc/passwd
❯ printf 'n\n' | script -qec "/tmp/opencode/mv-pr15294 src /etc/passwd" /dev/null
n
mv-pr15294: overwrite '/etc/passwd'? ⏎                                                                                                                                                    ❯ # but gnu
❯ script -qec "mv src /etc/passwd"
mv: replace '/etc/passwd', overriding mode 0644 (rw-r--r--)? ^C⏎
❯ # wording is off!

@anastygnome

Copy link
Copy Markdown
Contributor

OK, that's a separate issue! I'll focus on the technical aspects

This branch has not been deployed

No deployments
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.

3 participants