-
Notifications
You must be signed in to change notification settings - Fork 18
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
Script fails as user due to input redirection issues #5
Comments
Hmm I'll look into it Usually I just Perhaps I should just remove all the sudo stuff and force it into config-only mode if run as user… Might make more sense? |
Honestly, I'm not sure really. One could argue creating a config file as user makes sense, but then storing it in $XDG_CONFIG_HOME when you don't actually use it doesn't really… normally network settings and routes are root stuff so maybe just dumping the config in $PWD with a comment that it can be moved to /etc/ makes sense? I haven't fully thought this through so take with a grain of salt |
Currently, if the script is run with If it's run as root (without If it's run as user without Perhaps it should just assume I could absolutely fix all the sudo calls to do the do, but is that the appropriate choice? I'm thinking no at this point |
I tend to agree with your assessment. Setting routes and other network settings is a root task and as such the script should just bail if running as a user and asked to do more than drop a config. Where I don't necessarily agree is with creating a script in ~/.config/pia-wg/. "$XDG_CONFIG_HOME defines the base directory relative to which user-specific configuration files should be stored." However, in this case, this isn't a user-specific configuration files as it is supposed to be used by a system service. It makes some sense to drop it there if you allow the script to be run as user. IMHO it should drop the generated config file in $PWD, but I'm kind of nitpicking here. |
Yeah, as you can see your issue has unpacked a whole can of worms - which is good, I'm not adverse to a warranted ass-kicking ;) At this point I'm inclined to resolve it by making "run as user" → "implicit -c" with a notification on the terminal output to this effect. I think that would be the most sensible way to fix my previous lack of attention to this (admitted) mess.
That's an interesting idea, but I'm not fully onboard with it - my personal usage case for user-mode run is just to feed the output from Would it be acceptable to add an option flag somewhere for output location, and assume The default won't be |
I'm in no way an authority regarding where files should be stored… I was under the impression that the current behavior is to output the config both to stdout and into An option to specify an output path would be nice, but not strictly needed, in fact I'd personally say only stdout is fine (again this is personal preference) so in case you need both a config file and a qr code you could do something like btw I don't mean to tell you how to solve this particular issue, consider this brainstorming - it's your tool and I'm just trying to show some possibilities, but I'll be happy regardless of approach chosen for this tiny problem. |
The script fails to establish the connection to PIA due to input redirection issues.
Any lines that make use of it with sudo fail (e.g.
pia-wg/pia-wg.sh
Line 487 in a5a68d1
On this machine, the issue can be observed easily:
opens /dev/fd/63 with the content "test".
opens the same file, but it appears empty. To run the command with sudo successfully, this one works:
spawning a full subshell with elevated privileges, however this requires escaping any quotation marks.
I don't really have an easy solution for the problem, but this script as is doesn't work here.
Versions:
sudo: 1.9.11p3
wg: v1.0.20210914
bash: 5.1.16(1)
The text was updated successfully, but these errors were encountered: