Shell: Unsetting PATH doesn't make Shell unable to find things

[ADKaster]

From Ubuntu 21.04:

andrew@Andrew-Workstation:~/serenity$ ls
AK  azure-pipelines.yml  Base  Build  CMakeLists.txt  CONTRIBUTING.md  Documentation  Kernel  LICENSE  Meta  Ports  README.md  Tests  Toolchain  Userland
andrew@Andrew-Workstation:~/serenity$ unset PATH
andrew@Andrew-Workstation:~/serenity$ ls
bash: ls: No such file or directory
andrew@Andrew-Workstation:~/serenity$ 

And from Serenity:

Note that processes spawned from Shell are unable to find things, but Shell itself doesn't reset its PATH when unsetting shell variables that happen to be its own env vars (maybe?).

[alimpfard]

I'm not honestly sure if this is worth "fixing". To fix this, we need to either

• Not use execvp[e] as it has a default for PATH (meaning: implement our own PATH lookup)
• Or set PATH to something like : upon unset PATH to work around that default

Note that the default value is present on glibc as well.

[alimpfard]

For reference, the "fix" would look like this, effectively duplicating execvpe without the default:

diff --git a/Userland/Shell/Shell.cpp b/Userland/Shell/Shell.cpp
index 8e7dd82ed6..670bea4ac4 100644
--- a/Userland/Shell/Shell.cpp
+++ b/Userland/Shell/Shell.cpp
@@ -845,7 +845,14 @@ ErrorOr<RefPtr<Job>> Shell::run_command(const AST::Command& command)

 void Shell::execute_process(Vector<const char*>&& argv)
 {
-    int rc = execvp(argv[0], const_cast<char* const*>(argv.data()));
+    int rc = -1;
+    errno = ENOENT;
+    for (auto path : StringView { getenv("PATH") }.split_view(':')) {
+        LexicalPath lexical_path { path };
+        rc = execvp(lexical_path.append(argv[0]).string().characters(), const_cast<char* const*>(argv.data()));
+        if (rc < 0 && errno != ENOENT)
+            break;
+    }
     if (rc < 0) {
         auto parts = StringView { argv[0] }.split_view('/');
         if (parts.size() == 1) {

I'll put this in a PR so we can discuss it there :shrug: