chown: allow setting arbitrary numeric user ID

[jfinkels]

Help needed: the test fails because it requires root privileges to change the owner of a file. I couldn't quite follow how the other tests in this file are accomplishing this. The test passes if you run it as root. Any recommendations for what to do about this? Just skip the test if the user is not root?

---

This pull request updates chown to allow setting the owner of a file to a numeric user ID regardless of whether a corresponding username exists on the system.

For example,

$ touch f && sudo chown 12345 f

succeeds even though there is no named user with ID 12345.

Fixes #3380.

[jhscheer]

Any recommendations for what to do about this? Just skip the test if the user is not root?

Haven't looked into it recently but as far as I remember yes.
I tried to make this more convenient here #2692

[jfinkels]

I've updated this pull request to use the run_ucmd_as_root() helper function introduced in pull request #2692. Let's hope it works 🤞

[jfinkels]

Hmm something's still not quite right:

---- test_chown::test_chown_only_user_id stdout ----
current_directory_resolved: 
run: id -u
touch: /tmp/.tmpzLMcnj/test_chown_file1
run: /target/x86_64-unknown-linux-gnu/debug/coreutils chown 1001 --verbose test_chown_file1
result.stdout = 
result.stderr = thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Custom { kind: NotFound, error: "No such id: 1001" }', src/uucore/src/lib/features/perms.rs:153:48
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

thread 'test_chown::test_chown_only_user_id' panicked at 'Command was expected to succeed.
stdout = 
 stderr = thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Custom { kind: NotFound, error: "No such id: 1001" }', src/uucore/src/lib/features/perms.rs:153:48
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
', tests/common/util.rs:174:9

---- test_chown::test_chown_only_user_id_nonexistent_user stdout ----
current_directory_resolved: 
touch: /tmp/.tmpCsFABD/f
run: sudo -E --non-interactive whoami
thread 'test_chown::test_chown_only_user_id_nonexistent_user' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }', tests/common/util.rs:1070:14

---- test_chown::test_chown_owner_group_id stdout ----
current_directory_resolved: 
run: id -u
run: id -g
touch: /tmp/.tmpIfke8U/test_chown_file1
run: /target/x86_64-unknown-linux-gnu/debug/coreutils chown 1001:121 --verbose test_chown_file1
result.stdout = 
result.stderr = chown: invalid group: '1001:121'

thread 'test_chown::test_chown_owner_group_id' panicked at 'Command was expected to succeed.
stdout = 
 stderr = chown: invalid group: '1001:121'
', tests/common/util.rs:174:9


failures:
    test_chown::test_chown_only_user_id
    test_chown::test_chown_only_user_id_nonexistent_user
    test_chown::test_chown_owner_group_id

[jfinkels]

Executive summary: I believe my change revealed an issue in the perms.rs module in uucore. I'll fix that issue in a separate pull request here.

Details: Let's take a look at one of the failures, test_chown::test_chown_only_user_id stdout:

According to the logged messages, the test method first calls id -u to get the ID of the current user. This seems to return 1001 in the Ubuntu CI environment. Then it creates a file named test_chown_file1. Finally it runs chown 1001 --verbose test_chown_file1, which panics.

I think that before the change I'm proposing in this pull request, chown terminated with an error message like invalid user: 1001, which caused the test to be skipped due to this block:

coreutils/tests/by-util/test_chown.rs

Lines 404 to 409 in e5d9737

	let result = scene.ucmd().arg(user_id).arg("--verbose").arg(file1).run();
	if skipping_test_is_okay(&result, "invalid user") {
	// From the Logs: "Build (ubuntu-18.04, x86_64-unknown-linux-gnu, feat_os_unix, use-cross)"
	// stderr: "chown: invalid user: '1001'
	return;
	}

The panic is happening now because the command-line argument 1001 is (correctly) interpreted as a user ID, and chown attempts to set the owner of the file to the user with that ID. That code is ultimately in perms.rs in uucore. When that code tries to produce the info message required by the --verbose flag, it assumes (by way of an unwrap() call) that the given user ID always corresponds to a named user:

coreutils/src/uucore/src/lib/features/perms.rs

Lines 150 to 155 in e5d9737

	format!(
	"ownership of {} retained as {}:{}",
	path.quote(),
	entries::uid2usr(dest_uid).unwrap(),
	entries::gid2grp(dest_gid).unwrap()
	)

The assumption that a user ID always corresponds to a named user is incorrect. Here's an example of how chown can work with a nameless user ID:

$ touch f && sudo chown 12345 f && sudo chown --verbose 12345 f
ownership of 'f' retained as 12345

However, the username still appears in the verbose message even if it is given as a user ID in the argument:

$ touch f && chown --verbose $(id -u) f
ownership of 'f' retained as jeffrey

[jfinkels]

I added a commit that updates the perms.rs module.

[jfinkels]

It looks like my changes to perms.rs resolved the test failure in test_chown_only_user_id, but two failures still remain:

failures:

---- test_chown::test_chown_only_user_id_nonexistent_user stdout ----
current_directory_resolved: 
touch: /tmp/.tmplmvqpC/f
run: sudo -E --non-interactive whoami
thread 'test_chown::test_chown_only_user_id_nonexistent_user' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }', tests/common/util.rs:1070:14

---- test_chown::test_chown_owner_group_id stdout ----
current_directory_resolved: 
run: id -u
run: id -g
touch: /tmp/.tmpsWIZLg/test_chown_file1
run: /target/x86_64-unknown-linux-gnu/debug/coreutils chown 1001:121 --verbose test_chown_file1
result.stdout = 
result.stderr = chown: invalid group: '1001:121'

thread 'test_chown::test_chown_owner_group_id' panicked at 'Command was expected to succeed.
stdout = 
 stderr = chown: invalid group: '1001:121'
', tests/common/util.rs:174:9


failures:
    test_chown::test_chown_only_user_id_nonexistent_user
    test_chown::test_chown_owner_group_id

I'll diagnose them later this week.

[jfinkels]

I'm trying to use run_umcd_as_root(), but I'm getting the following error output

---- test_chown::test_chown_only_group_id_nonexistent_group stdout ----
current_directory_resolved: 
touch: /tmp/.tmpM827bl/f
run: sudo -E --non-interactive whoami
thread 'test_chown::test_chown_only_group_id_nonexistent_group' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }', tests/common/util.rs:1070:14

I think this is saying that it tried to run sudo -E --non-interactive whoami but the call to Command::spawn() returned an error, namely a "No such file or directory" error. Could it be that whoami is not accessible from the CI environment? @jhscheer do you have any ideas?

[jhscheer]

I think this is saying that it tried to run sudo -E --non-interactive whoami but the call to Command::spawn() returned an error, namely a "No such file or directory" error. Could it be that whoami is not accessible from the CI environment? @jhscheer do you have any ideas?

Could be whoami, but my guess is, that it's sudo that's missing.

Sorry run_umcd_as_root isn't very technically mature, yet.

I see three options to improve it:

1. bubble up the error from fn run_no_wait(&mut self) -> Child to fn run_no_wait(&mut self) -> Result<Child, Error>
2. use fn is_ci() inside fn run_ucmd_as_root to not run .cmd_keepenv("sudo") if inside ci
3. use process::Command::new inside fn run_ucmd_as_root to bypass fn run_no_wait, similar to what is done in fn test_run_ucmd_as_root

Edit
I implemented 3, please let me know if it helps.
#3518