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

Resholve doesn’t understand sudo --verison #113

Open
Jayman2000 opened this issue Feb 15, 2024 · 4 comments
Open

Resholve doesn’t understand sudo --verison #113

Jayman2000 opened this issue Feb 15, 2024 · 4 comments

Comments

@Jayman2000
Copy link
Contributor

Resholve’s parser for sudo is supposed to support sudo --version, but it doesn’t work. This Nix expression:

{ pkgs ? import <nixpkgs> { } }:
pkgs.resholve.writeScript "sudo-fix-example" {
  inputs = [ pkgs.sudo ];
  fix = [ "sudo" ];
  interpreter = "${pkgs.bash}/bin/bash";
} ''
  sudo --version
''

fails to build with the following error:

[resholve context] : invoking resholve with PWD=/build
[resholve context] RESHOLVE_LORE=/nix/store/yhc5n7zv9sw87nbfn3qhnv05jifvpj76-more-binlore
[resholve context] RESHOLVE_FIX=sudo
[resholve context] RESHOLVE_INPUTS=/nix/store/q56d1m0ws1kf8vddfw71d8nfagnv0x6g-sudo-1.9.15p2/bin
[resholve context] RESHOLVE_INTERPRETER=/nix/store/7dpxg7ki7g8ynkdwcqf493p2x8divb4i-bash-5.2-p15/bin/bash
[resholve context] /nix/store/c99gv3kyb0qzqhppsb4iqijnzqx475vh-resholve-0.9.0/bin/resholve --overwrite /nix/store/bqah3m79k7wyrq3cah4sjbxwppm42xqk-sudo-fix-example
WARNING:__main__:CommandParser CommandParser(prog='sudo (generic)', usage=None, description=None, version=None, formatter_class=<class 'argparse.HelpFormatter'>, conflict_handler='error', add_help=False) passing instead of 'too few arguments'
WARNING:__main__:CommandParser CommandParser(prog='sudo (generic)', usage=None, description=None, version=None, formatter_class=<class 'argparse.HelpFormatter'>, conflict_handler='error', add_help=False) passing instead of 'argument commands is required'
  sudo --version
  ^~~~
'/nix/store/bqah3m79k7wyrq3cah4sjbxwppm42xqk-sudo-fix-example':2: 'sudo' executes its arguments in some circumstances, and none of my command-specific rules for figuring out if this specific invocation does or not succeeded. Look above for warning messages with more context on what went wrong.
@abathur
Copy link
Owner

abathur commented Feb 15, 2024

Egh :)

My kingdom for a cli syntax parser that's a better fit for what we need.

I think the positional arg is causing this. Probably need to make a 2nd parser for sudo to catch the forms w/o a positional arg.

Jayman2000 added a commit to Jayman2000/jasons-nixos-config that referenced this issue Feb 15, 2024
For whatever reason, when resholve encounters something like this:

	sudo echo hi

it doesn’t resolve the echo command. I’m not sure why it doesn’t resolve
that command, but I have a feeling that it’s related to this issue [1].

This commit works around that resholve limitation by pre-resolving
commands that get executed by sudo. The main motivation behind this
commit is to make creating a future commit easier. In order for that
future commit to work, this command:

	sudo nixos-install

must be transformed into this command:

	sudo <path-to-nixos-install>

because nixos-install won’t be on the PATH.

[1]: <abathur/resholve#113>

TODO:
• Add note referencing the future commit.
Jayman2000 added a commit to Jayman2000/jasons-nixos-config that referenced this issue Feb 15, 2024
For whatever reason, when resholve encounters something like this:

	sudo echo hi

it doesn’t resolve the echo command. I’m not sure why it doesn’t resolve
that command, but I have a feeling that it’s related to this issue [1].

This commit works around that resholve limitation by pre-resolving
commands that get executed by sudo. The main motivation behind this
commit is to make creating a future commit easier. In order for that
future commit to work, this command:

	sudo nixos-install

must be transformed into this command:

	sudo <path-to-nixos-install>

because nixos-install won’t be on the PATH.

[1]: <abathur/resholve#113>
abathur added a commit that referenced this issue Feb 16, 2024
Will hopefully fix #113. The attempt to model one of sudo's
forms with dest="skip" was a mistake, and we weren't testing
sudo well enough (because of nix/nixpkgs issues here) for it
to get caught quickly.
@abathur
Copy link
Owner

abathur commented Feb 16, 2024

I pushed a tentative fix in 39f5012 if you're able to test it?

@Jayman2000
Copy link
Contributor Author

It looks like that tentative fix works, but I think that I’ve discovered another problem. If I use RESHOLVE_FIX=sudo, then repo/tests/parse_sudo.sh becomes this:

#!/nix/store/f2ibj8np6dyddf3kq68dkcvwfr6bmi9n-bash-interactive-5.2-p15/bin/bash
# Caution: you're faking sudo as a script that runs `true`
/nix/store/q56d1m0ws1kf8vddfw71d8nfagnv0x6g-sudo-1.9.15p2/bin/sudo --version
/nix/store/q56d1m0ws1kf8vddfw71d8nfagnv0x6g-sudo-1.9.15p2/bin/sudo -v
/nix/store/q56d1m0ws1kf8vddfw71d8nfagnv0x6g-sudo-1.9.15p2/bin/sudo /nix/store/m38gwq0w8w7qyjn9s00balyp7cv3m5p9-coreutils-9.3/bin/ls

### resholve directives (auto-generated) ## format_version: 3
# resholve: fix sudo
# resholve: keep /nix/store/m38gwq0w8w7qyjn9s00balyp7cv3m5p9-coreutils-9.3/bin/ls
# resholve: keep /nix/store/q56d1m0ws1kf8vddfw71d8nfagnv0x6g-sudo-1.9.15p2/bin/sudo

But, if I use RESHOLVE_FAKE=external:sudo then repo/tests/parse_sudo.sh becomes this:

#!/nix/store/f2ibj8np6dyddf3kq68dkcvwfr6bmi9n-bash-interactive-5.2-p15/bin/bash
# Caution: you're faking sudo as a script that runs `true`
sudo --version
sudo -v
sudo ls

### resholve directives (auto-generated) ## format_version: 3
# resholve: fake external:sudo

Shouldn’t ls have been resolved, or am I misunderstanding how fake directives are supposed to work?

@abathur
Copy link
Owner

abathur commented Feb 23, 2024

Shouldn’t ls have been resolved, or am I misunderstanding how fake directives are supposed to work?

As implemented, fake directives preempt resolution, and sub-exec is part of resolution.

That said, fake directives were implemented before sub-exec resolution, and I agree there's potential for confusion here.

I'm tempted to say these might both deserve independent levers, though I'm not sure how much work it is and there are some confounding variables like whether faking a function/alias sudo should pull in sub-exec processing or not...

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

No branches or pull requests

2 participants