Cleanup tahoe module #25271
Cleanup tahoe module #25271
Conversation
- Remove useless escape of question mark - Fix and quoting - Add some '&&s' for correctness
|
@0xABAB, thanks for your PR! By analyzing the history of the files in this pull request, we identified @MostAwesomeDude, @ericsagnes and @fpletz to be potential reviewers. |
|
Can you elaborate on how |
|
@joachifm Using |
|
The |
|
@joachifm I was not aware of that, but I also don't want to be aware of that. Global assumptions are bad. I consider running with |
| @@ -237,22 +237,22 @@ in | |||
| # arguments to $(tahoe start). The node directory must come first, | |||
| # and arguments which alter Twisted's behavior come afterwards. | |||
| ExecStart = '' | |||
| ${settings.package}/bin/tahoe start ${nodedir} -n -l- --pidfile=${pidfile} | |||
| ${settings.package}/bin/tahoe start "${nodedir}" -n -l- --pidfile="${pidfile}" | |||
There was a problem hiding this comment.
If nodedir or pidfile include " or $, this still won't be interpreted correctly. If we really want to fix this properly, I think it would be better to use escapeShellArg in the Nix interpolation, e.g.:
${lib.escapeShellArg pidfile} (without double-quotes around it, and assuming lib is in scope). I'm not sure if it's worth the verbosity, but that would be the most solid way to do it.
Make sure people can't inject shell code by accident.
|
@ryantrinkle Thanks for your contribution. |
|
@joachifm I made the requested changes and then some. Please merge. |
| mkdir -p /var/db/tahoe-lafs | ||
| tahoe create-introducer ${nodedir} | ||
| tahoe create-introducer "{lib.escapeShellArg nodedir} |
|
Applied with a patch in 90acbe5 |
Motivation for this change
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandboxinnix.confon non-NixOS)
nix-shell -p nox --run "nox-review wip"./result/bin/)